Uses a built-in method to handle plural exception cases

Implements check for empty segments
Implements check for WP segment when unsubscribing

lib/API/MP/v1/API.php

@@ -42,8 +42,12 @@ class API {
   }
 
   function subscribeToLists($subscriber_id, array $segments_ids) {
-    $subscriber = Subscriber::findOne($subscriber_id);
+    if(empty($segments_ids)) {
+      throw new \Exception(__('At least one segment ID is required.', 'mailpoet'));
+    }
+
     // throw exception when subscriber does not exist
+    $subscriber = Subscriber::findOne($subscriber_id);
     if(!$subscriber) {
       throw new \Exception(__('This subscriber does not exist.', 'mailpoet'));
     }
@@ -51,9 +55,7 @@ class API {
     // throw exception when none of the segments exist
     $found_segments = Segment::whereIn('id', $segments_ids)->findMany();
     if(!$found_segments) {
-      $exception = (count($segments_ids) === 1) ?
+      $exception = _n('This list does not exist.', 'These lists do not exist.', count($segments_ids), 'mailpoet');
-        __('This list does not exist.', 'mailpoet') :
-        __('These lists do not exist.', 'mailpoet');
       throw new \Exception($exception);
     }
 
@@ -69,9 +71,10 @@ class API {
     // throw an exception when one or more segments do not exist
     if(count($found_segments_ids) !== count($segments_ids)) {
       $missing_ids = array_values(array_diff($segments_ids, $found_segments_ids));
-      $exception = (count($missing_ids) === 1) ?
+      $exception = sprintf(
-        __('List with ID %s does not exist.', 'mailpoet') :
+        _n('List with ID %s does not exist.', 'Lists with IDs %s do not exist.', count($missing_ids), 'mailpoet'),
-        __('Lists with IDs %s do not exist.', 'mailpoet');
+        implode(', ', $missing_ids)
+      );
       throw new \Exception(sprintf($exception, implode(', ', $missing_ids)));
     }
 
@@ -84,8 +87,12 @@ class API {
   }
 
   function unsubscribeFromLists($subscriber_id, array $segments_ids) {
-    $subscriber = Subscriber::findOne($subscriber_id);
+    if(empty($segments_ids)) {
+      throw new \Exception(__('At least one segment ID is required.', 'mailpoet'));
+    }
+
     // throw exception when subscriber does not exist
+    $subscriber = Subscriber::findOne($subscriber_id);
     if(!$subscriber) {
       throw new \Exception(__('This subscriber does not exist.', 'mailpoet'));
     }
@@ -93,24 +100,26 @@ class API {
     // throw exception when none of the segments exist
     $found_segments = Segment::whereIn('id', $segments_ids)->findMany();
     if(!$found_segments) {
-      $exception = (count($segments_ids) === 1) ?
+      $exception = _n('This list does not exist.', 'These lists do not exist.', count($segments_ids), 'mailpoet');
-        __('This list does not exist.', 'mailpoet') :
-        __('These lists do not exist.', 'mailpoet');
       throw new \Exception($exception);
     }
 
-    // throw exception when trying to unsubscribe from a WP Users segment
+    // throw exception when trying to subscribe to a WP Users segment
     $found_segments_ids = array_map(function($segment) {
+      if($segment->type === Segment::TYPE_WP_USERS) {
+        throw new \Exception(__(sprintf("Can't subscribe to a WordPress Users list with ID %d.", $segment->id), 'mailpoet'));
+      }
       return $segment->id;
     }, $found_segments);
 
     // throw an exception when one or more segments do not exist
     if(count($found_segments_ids) !== count($segments_ids)) {
       $missing_ids = array_values(array_diff($segments_ids, $found_segments_ids));
-      $exception = (count($missing_ids) === 1) ?
+      $exception = sprintf(
-        __('List with ID %s does not exist.', 'mailpoet') :
+        _n('List with ID %s does not exist.', 'Lists with IDs %s do not exist.', count($missing_ids), 'mailpoet'),
-        __('Lists with IDs %s do not exist.', 'mailpoet');
+        implode(', ', $missing_ids)
-      throw new \Exception(sprintf($exception, implode(', ', $missing_ids)));
+      );
+      throw new \Exception($exception);
     }
 
     SubscriberSegment::unsubscribeFromSegments($subscriber, $found_segments_ids);

tests/unit/API/MP/APITest.php

@@ -134,22 +134,6 @@ class APITest extends \MailPoetTest {
     );
   }
 
-  function testItSubscribesSubscriberToSingleList() {
-    $subscriber = Subscriber::create();
-    $subscriber->hydrate(Fixtures::get('subscriber_template'));
-    $subscriber->save();
-    $segment = Segment::createOrUpdate(
-      array(
-        'name' => 'Default',
-        'type' => Segment::TYPE_DEFAULT
-      )
-    );
-    $result = API::MP(self::VERSION)->subscribeToList($subscriber->id, $segment->id);
-    expect($result['id'])->equals($subscriber->id);
-    expect($result['subscriptions'])->notEmpty();
-    expect($result['subscriptions'][0]['segment_id'])->equals($segment->id);
-  }
-
   function testItSubscribesSubscriberToMultipleLists() {
     $subscriber = Subscriber::create();
     $subscriber->hydrate(Fixtures::get('subscriber_template'));
@@ -160,6 +144,15 @@ class APITest extends \MailPoetTest {
         'type' => Segment::TYPE_DEFAULT
       )
     );
+
+    // test if segments are specified
+    try {
+      API::MP(self::VERSION)->subscribeToLists($subscriber->id, array());
+      $this->fail('Segments are required exception should have been thrown.');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals('At least one segment ID is required.');
+    }
+
     $result = API::MP(self::VERSION)->subscribeToLists($subscriber->id, array($segment->id));
     expect($result['id'])->equals($subscriber->id);
     expect($result['subscriptions'][0]['segment_id'])->equals($segment->id);
@@ -440,6 +433,24 @@ class APITest extends \MailPoetTest {
     }
   }
 
+  function testItDoesNotUnsubscribeSubscriberFromWPUsersList() {
+    $subscriber = Subscriber::create();
+    $subscriber->hydrate(Fixtures::get('subscriber_template'));
+    $subscriber->save();
+    $segment = Segment::createOrUpdate(
+      array(
+        'name' => 'Default',
+        'type' => Segment::TYPE_WP_USERS
+      )
+    );
+    try {
+      API::MP(self::VERSION)->unsubscribeFromLists($subscriber->id, array($segment->id));
+      $this->fail('WP Users segment exception should have been thrown.');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals("Can't subscribe to a WordPress Users list with ID {$segment->id}.");
+    }
+  }
+
   function testItUsesMultipleListsUnsubscribeMethodWhenUnsubscribingFromSingleList() {
     // unsubscribing from single list = converting list ID to an array and using
     // multiple lists unsubscribe method
@@ -468,6 +479,15 @@ class APITest extends \MailPoetTest {
         'type' => Segment::TYPE_DEFAULT
       )
     );
+
+    // test if segments are specified
+    try {
+      API::MP(self::VERSION)->unsubscribeFromLists($subscriber->id, array());
+      $this->fail('Segments are required exception should have been thrown.');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals('At least one segment ID is required.');
+    }
+
     $result = API::MP(self::VERSION)->subscribeToLists($subscriber->id, array($segment->id));
     expect($result['subscriptions'][0]['status'])->equals(Subscriber::STATUS_SUBSCRIBED);
     $result = API::MP(self::VERSION)->unsubscribeFromLists($subscriber->id, array($segment->id));