Improves error messages, cleans up code

lib/API/MP/v1/API.php

@@ -51,7 +51,10 @@ class API {
     // throw exception when none of the segments exist
     $found_segments = Segment::whereIn('id', $segments_ids)->findMany();
     if(!$found_segments) {
-      throw new \Exception(__('These lists do not exist.', 'mailpoet'));
+      $exception = (count($segments_ids) === 1) ?
+        __('This list does not exist.', 'mailpoet') :
+        __('These lists do not exist.', 'mailpoet');
+      throw new \Exception($exception);
     }
 
     // throw exception when trying to subscribe to a WP Users segment
@@ -66,7 +69,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));
-      throw new \Exception(__(sprintf('Lists with ID %s do not exist.', implode(', ', $missing_ids)), 'mailpoet'));
+      $exception = (count($missing_ids) === 1) ?
+        __('List with ID %s does not exist.', 'mailpoet') :
+        __('Lists with IDs %s do not exist.', 'mailpoet');
+      throw new \Exception(sprintf($exception, implode(', ', $missing_ids)));
     }
 
     SubscriberSegment::subscribeToSegments($subscriber, $found_segments_ids);

tests/unit/API/MP/APITest.php

@@ -1,4 +1,5 @@
 <?php
+
 namespace MailPoet\Test\API\MP;
 
 use Codeception\Util\Fixtures;
@@ -55,12 +56,20 @@ class APITest extends \MailPoetTest {
     $subscriber = Subscriber::create();
     $subscriber->hydrate(Fixtures::get('subscriber_template'));
     $subscriber->save();
+    // multiple lists error message
     try {
       API::MP(self::VERSION)->subscribeToLists($subscriber->id, array(1,2,3));
       $this->fail('Missing segments exception should have been thrown.');
     } catch(\Exception $e) {
       expect($e->getMessage())->equals('These lists do not exist.');
     }
+    // single list error message
+    try {
+      API::MP(self::VERSION)->subscribeToLists($subscriber->id, array(1));
+      $this->fail('Missing segments exception should have been thrown.');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals('This list does not exist.');
+    }
   }
 
   function testItDoesNotSubscribeSubscriberToWPUsersList() {
@@ -81,7 +90,7 @@ class APITest extends \MailPoetTest {
     }
   }
 
-  function testItDoesNotSubscribeSubscriberToListsWhenOneOrMostListsAreMissing() {
+  function testItDoesNotSubscribeSubscriberToListsWhenOneOrMoreListsAreMissing() {
     $subscriber = Subscriber::create();
     $subscriber->hydrate(Fixtures::get('subscriber_template'));
     $subscriber->save();
@@ -91,27 +100,38 @@ class APITest extends \MailPoetTest {
         'type' => Segment::TYPE_DEFAULT
       )
     );
+    // multiple lists error message
     try {
       API::MP(self::VERSION)->subscribeToLists($subscriber->id, array($segment->id, 90, 100));
       $this->fail('Missing segments with IDs exception should have been thrown.');
     } catch(\Exception $e) {
-      expect($e->getMessage())->equals('Lists with ID 90, 100 do not exist.');
+      expect($e->getMessage())->equals('Lists with IDs 90, 100 do not exist.');
+    }
+    // single list error message
+    try {
+      API::MP(self::VERSION)->subscribeToLists($subscriber->id, array($segment->id, 90));
+      $this->fail('Missing segments with IDs exception should have been thrown.');
+    } catch(\Exception $e) {
+      expect($e->getMessage())->equals('List with ID 90 does not exist.');
     }
   }
 
-  function testItSubscribesSubscriberToMultupleLists() {
+  function testItUsesMultipleListsSubscribeMethodWhenSubscribingToSingleList() {
-    $subscriber = Subscriber::create();
+    // subscribing to single list = converting list ID to an array and using
-    $subscriber->hydrate(Fixtures::get('subscriber_template'));
+    // multiple lists subscription method
-    $subscriber->save();
+    $API = Stub::make(new \MailPoet\API\MP\v1\API(), array(
-    $segment = Segment::createOrUpdate(
+      'subscribeToLists' => function() {
+        return func_get_args();
+      }
+    ));
+    expect($API->subscribeToList(1,2))->equals(
       array(
-        'name' => 'Default',
+        1,
-        'type' => Segment::TYPE_DEFAULT
+        array(
+          2
+        )
       )
     );
-    $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);
   }
 
   function testItSubscribesSubscriberToSingleList() {
@@ -130,6 +150,21 @@ class APITest extends \MailPoetTest {
     expect($result['subscriptions'][0]['segment_id'])->equals($segment->id);
   }
 
+  function testItSubscribesSubscriberToMultipleLists() {
+    $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)->subscribeToLists($subscriber->id, array($segment->id));
+    expect($result['id'])->equals($subscriber->id);
+    expect($result['subscriptions'][0]['segment_id'])->equals($segment->id);
+  }
+
   function testItSubscribesSubscriberWithEmailIdentifier() {
     $subscriber = Subscriber::create();
     $subscriber->hydrate(Fixtures::get('subscriber_template'));