Refactor MP API addSubscriber to doctrine

[MAILPOET-4293]

mailpoet/lib/API/MP/v1/API.php

@@ -4,9 +4,6 @@ namespace MailPoet\API\MP\v1;
 
 use MailPoet\Config\Changelog;
 use MailPoet\Models\Subscriber;
-use MailPoet\Subscribers\RequiredCustomFieldValidator;
-use MailPoet\Subscribers\Source;
-use MailPoet\Util\Helpers;
 
 /**
  * API used by other plugins
@@ -14,10 +11,6 @@ use MailPoet\Util\Helpers;
  * This class is under refactor, and we are going to move most of the remaining implementations from here.
  */
 class API {
-
-  /** @var RequiredCustomFieldValidator */
-  private $requiredCustomFieldValidator;
-
   /** @var CustomFields */
   private $customFields;
 
@@ -31,13 +24,11 @@ class API {
   private $changelog;
 
   public function __construct(
-    RequiredCustomFieldValidator $requiredCustomFieldValidator,
     CustomFields $customFields,
     Segments $segments,
     Subscribers $subscribers,
     Changelog $changelog
   ) {
-    $this->requiredCustomFieldValidator = $requiredCustomFieldValidator;
     $this->customFields = $customFields;
     $this->segments = $segments;
     $this->subscribers = $subscribers;
@@ -82,70 +73,8 @@ class API {
     return $this->segments->getAll();
   }
 
-  public function addSubscriber(array $subscriber, $listIds = [], $options = []) {
+  public function addSubscriber(array $subscriber, $listIds = [], $options = []): array {
-    $sendConfirmationEmail = (isset($options['send_confirmation_email']) && $options['send_confirmation_email'] === false) ? false : true;
+    return $this->subscribers->addSubscriber($subscriber, $listIds, $options);
-    $scheduleWelcomeEmail = (isset($options['schedule_welcome_email']) && $options['schedule_welcome_email'] === false) ? false : true;
-    $skipSubscriberNotification = (isset($options['skip_subscriber_notification']) && $options['skip_subscriber_notification'] === true) ? true : false;
-
-    // throw exception when subscriber email is missing
-    if (empty($subscriber['email'])) {
-      throw new APIException(
-        __('Subscriber email address is required.', 'mailpoet'),
-        APIException::EMAIL_ADDRESS_REQUIRED
-      );
-    }
-
-    // throw exception when subscriber already exists
-    if (Subscriber::findOne($subscriber['email'])) {
-      throw new APIException(
-        __('This subscriber already exists.', 'mailpoet'),
-        APIException::SUBSCRIBER_EXISTS
-      );
-    }
-
-    if (empty($subscriber['subscribed_ip'])) {
-      $subscriber['subscribed_ip'] = Helpers::getIP();
-    }
-
-    // separate data into default and custom fields
-    [$defaultFields, $customFields] = Subscriber::extractCustomFieldsFromFromObject($subscriber);
-
-    // filter out all incoming data that we don't want to change, like status ...
-    $defaultFields = array_intersect_key($defaultFields, array_flip(['email', 'first_name', 'last_name', 'subscribed_ip']));
-
-    // if some required default fields are missing, set their values
-    $defaultFields = Subscriber::setRequiredFieldsDefaultValues($defaultFields);
-
-    $this->requiredCustomFieldValidator->validate($customFields);
-
-    // add subscriber
-    $newSubscriber = Subscriber::create();
-    $newSubscriber->hydrate($defaultFields);
-    $newSubscriber = Source::setSource($newSubscriber, Source::API);
-    $newSubscriber->save();
-    if ($newSubscriber->getErrors() !== false) {
-      throw new APIException(
-        // translators: %s is a comma-seperated list of errors.
-        sprintf(__('Failed to add subscriber: %s', 'mailpoet'), strtolower(implode(', ', $newSubscriber->getErrors()))),
-        APIException::FAILED_TO_SAVE_SUBSCRIBER
-      );
-    }
-    if (!empty($customFields)) {
-      $newSubscriber->saveCustomFields($customFields);
-    }
-
-    // reload subscriber to get the saved status/created|updated|delete dates/other fields
-    $newSubscriber = Subscriber::findOne($newSubscriber->id);
-
-    // subscribe to segments and optionally: 1) send confirmation email, 2) schedule welcome email(s)
-    if (!empty($listIds)) {
-      $this->subscribeToLists($newSubscriber->id, $listIds, [
-        'send_confirmation_email' => $sendConfirmationEmail,
-        'schedule_welcome_email' => $scheduleWelcomeEmail,
-        'skip_subscriber_notification' => $skipSubscriberNotification,
-      ]);
-    }
-    return $newSubscriber->withCustomFields()->withSubscriptions()->asArray();
   }
 
   public function addList(array $list) {

mailpoet/lib/API/MP/v1/Subscribers.php

@@ -11,9 +11,13 @@ use MailPoet\Segments\SegmentsRepository;
 use MailPoet\Settings\SettingsController;
 use MailPoet\Subscribers\ConfirmationEmailMailer;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
+use MailPoet\Subscribers\RequiredCustomFieldValidator;
+use MailPoet\Subscribers\Source;
+use MailPoet\Subscribers\SubscriberSaveController;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
 use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Tasks\Sending;
+use MailPoet\Util\Helpers;
 use MailPoet\WP\Functions as WPFunctions;
 
 class Subscribers {
@@ -44,9 +48,15 @@ class Subscribers {
   /** @var NewSubscriberNotificationMailer */
   private $newSubscriberNotificationMailer;
 
+  /** @var SubscriberSaveController */
+  private $subscriberSaveController;
+
   /** @var FeaturesController */
   private $featuresController;
 
+  /** @var RequiredCustomFieldValidator */
+  private $requiredCustomFieldsValidator;
+
   /** @var WPFunctions */
   private $wp;
 
@@ -57,9 +67,11 @@ class Subscribers {
     SettingsController $settings,
     SubscriberSegmentRepository $subscriberSegmentRepository,
     SubscribersRepository $subscribersRepository,
+    SubscriberSaveController $subscriberSaveController,
     SubscribersResponseBuilder $subscribersResponseBuilder,
     WelcomeScheduler $welcomeScheduler,
     FeaturesController $featuresController,
+    RequiredCustomFieldValidator $requiredCustomFieldsValidator,
     WPFunctions $wp
   ) {
     $this->confirmationEmailMailer = $confirmationEmailMailer;
@@ -68,12 +80,78 @@ class Subscribers {
     $this->settings = $settings;
     $this->subscribersSegmentRepository = $subscriberSegmentRepository;
     $this->subscribersRepository = $subscribersRepository;
+    $this->subscriberSaveController = $subscriberSaveController;
     $this->subscribersResponseBuilder = $subscribersResponseBuilder;
     $this->welcomeScheduler = $welcomeScheduler;
     $this->featuresController = $featuresController;
+    $this->requiredCustomFieldsValidator = $requiredCustomFieldsValidator;
     $this->wp = $wp;
   }
 
+  public function addSubscriber(array $data, array $listIds = [], array $options = []): array {
+    $sendConfirmationEmail = !(isset($options['send_confirmation_email']) && $options['send_confirmation_email'] === false);
+    $scheduleWelcomeEmail = !(isset($options['schedule_welcome_email']) && $options['schedule_welcome_email'] === false);
+    $skipSubscriberNotification = (isset($options['skip_subscriber_notification']) && $options['skip_subscriber_notification'] === true);
+
+    // throw exception when subscriber email is missing
+    if (empty($data['email'])) {
+      throw new APIException(
+        __('Subscriber email address is required.', 'mailpoet'),
+        APIException::EMAIL_ADDRESS_REQUIRED
+      );
+    }
+
+    // throw exception when subscriber already exists
+    if ($this->subscribersRepository->findOneBy(['email' => $data['email']])) {
+      throw new APIException(
+        __('This subscriber already exists.', 'mailpoet'),
+        APIException::SUBSCRIBER_EXISTS
+      );
+    }
+
+    [$defaultFields, $customFields] = $this->extractCustomFieldsFromFromSubscriberData($data);
+
+    $this->requiredCustomFieldsValidator->validate($customFields);
+
+    // filter out all incoming data that we don't want to change, like status ...
+    $defaultFields = array_intersect_key($defaultFields, array_flip(['email', 'first_name', 'last_name', 'subscribed_ip']));
+
+    if (empty($defaultFields['subscribed_ip'])) {
+      $defaultFields['subscribed_ip'] = Helpers::getIP();
+    }
+    $defaultFields['source'] = Source::API;
+
+    try {
+      $subscriberEntity = $this->subscriberSaveController->createOrUpdate($defaultFields, null);
+    } catch (\Exception $e) {
+      throw new APIException(
+      // translators: %s is an error message.
+        sprintf(__('Failed to add subscriber: %s', 'mailpoet'), $e->getMessage()),
+        APIException::FAILED_TO_SAVE_SUBSCRIBER
+      );
+    }
+
+    try {
+      $this->subscriberSaveController->updateCustomFields($customFields, $subscriberEntity);
+    } catch (\Exception $e) {
+      throw new APIException(
+      // translators: %s is an error message
+        sprintf(__('Failed to save subscriber custom fields: %s', 'mailpoet'), $e->getMessage()),
+        APIException::FAILED_TO_SAVE_SUBSCRIBER
+      );
+    }
+
+    // subscribe to segments and optionally: 1) send confirmation email, 2) schedule welcome email(s)
+    if (!empty($listIds)) {
+      $this->subscribeToLists($subscriberEntity->getId(), $listIds, [
+        'send_confirmation_email' => $sendConfirmationEmail,
+        'schedule_welcome_email' => $scheduleWelcomeEmail,
+        'skip_subscriber_notification' => $skipSubscriberNotification,
+      ]);
+    }
+    return $this->subscribersResponseBuilder->build($subscriberEntity);
+  }
+
   /**
    * @throws APIException
    */
@@ -281,4 +359,19 @@ class Subscribers {
 
     return $foundSegments;
   }
+
+  /**
+   * Splits subscriber data into two arrays with basic data (index 0) and custom fields data (index 1)
+   * @return array<int, array>
+   */
+  private function extractCustomFieldsFromFromSubscriberData($data): array {
+    $customFields = [];
+    foreach ($data as $key => $value) {
+      if (strpos($key, 'cf_') === 0) {
+        $customFields[$key] = $value;
+        unset($data[$key]);
+      }
+    }
+    return [$data, $customFields];
+  }
 }

mailpoet/tests/integration/API/MP/APITest.php

@@ -23,6 +23,7 @@ use MailPoet\Settings\SettingsController;
 use MailPoet\Subscribers\ConfirmationEmailMailer;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
 use MailPoet\Subscribers\RequiredCustomFieldValidator;
+use MailPoet\Subscribers\SubscriberSaveController;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
 use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Tasks\Sending;
@@ -59,9 +60,11 @@ class APITest extends \MailPoetTest {
       SettingsController::getInstance(),
       $this->diContainer->get(SubscriberSegmentRepository::class),
       $this->diContainer->get(SubscribersRepository::class),
+      $this->diContainer->get(SubscriberSaveController::class),
       $this->diContainer->get(SubscribersResponseBuilder::class),
       Stub::makeEmpty(WelcomeScheduler::class),
       $this->diContainer->get(FeaturesController::class),
+      $this->diContainer->get(RequiredCustomFieldValidator::class),
       $this->diContainer->get(WPFunctions::class)
     );
   }
@@ -71,7 +74,6 @@ class APITest extends \MailPoetTest {
       $subscriberActions = $this->getSubscribers();
     }
     return new API(
-      $this->diContainer->get(RequiredCustomFieldValidator::class),
       $this->diContainer->get(CustomFields::class),
       $this->diContainer->get(Segments::class),
       $subscriberActions,
@@ -146,7 +148,7 @@ class APITest extends \MailPoetTest {
     } catch (\Exception $e) {
       expect($e->getMessage())->stringContainsString('Failed to add subscriber:');
       // error message (converted to lowercase) returned by the model
-      expect($e->getMessage())->stringContainsString('your email address is invalid!');
+      expect($e->getMessage())->stringContainsString('value is not a valid email address.');
     }
   }
 
@@ -193,6 +195,7 @@ class APITest extends \MailPoetTest {
     ];
 
     $this->expectException('Exception');
+    $this->expectExceptionMessage('Missing value for custom field "custom field');
     $this->getApi()->addSubscriber($subscriber);
   }
 
@@ -222,15 +225,16 @@ class APITest extends \MailPoetTest {
         'segmentsRepository' => $this->diContainer->get(SegmentsRepository::class),
         'subscribersRepository' => $this->diContainer->get(SubscribersRepository::class),
         'subscribersSegmentRepository' => $this->diContainer->get(SubscriberSegmentRepository::class),
+        'subscriberSaveController' => $this->diContainer->get(SubscriberSaveController::class),
         'subscribersResponseBuilder' => $this->diContainer->get(SubscribersResponseBuilder::class),
         'settings' => $settings,
+        'requiredCustomFieldsValidator' => Stub::makeEmpty(RequiredCustomFieldValidator::class, ['validate']),
       ],
       $this);
 
     $API = Stub::make(
       API::class,
       [
-        'requiredCustomFieldValidator' => Stub::makeEmpty(RequiredCustomFieldValidator::class, ['validate']),
         'subscribers' => $subscriberActions,
       ],
       $this
@@ -324,15 +328,27 @@ class APITest extends \MailPoetTest {
   }
 
   public function testByDefaultItSendsConfirmationEmailAfterAddingSubscriber() {
+    $subscriberActions = Stub::make(
+      Subscribers::class,
+      [
+        'segmentsRepository' => $this->diContainer->get(SegmentsRepository::class),
+        'subscribersRepository' => $this->diContainer->get(SubscribersRepository::class),
+        'subscriberSaveController' => $this->diContainer->get(SubscriberSaveController::class),
+        'subscribersResponseBuilder' => $this->diContainer->get(SubscribersResponseBuilder::class),
+        'settings' => $this->diContainer->get(SettingsController::class),
+        'requiredCustomFieldsValidator' => Stub::makeEmpty(RequiredCustomFieldValidator::class, ['validate']),
+        'subscribeToLists' => Expected::once(function ($subscriberId, $segmentsIds, $options) {
+          expect($options)->contains('send_confirmation_email');
+          expect($options['send_confirmation_email'])->equals(true);
+          return [];
+        })
+      ],
+      $this);
     $API = $this->makeEmptyExcept(
       API::class,
       'addSubscriber',
       [
-        'subscribeToLists' => Expected::once(function ($subscriberId, $segmentsIds, $options) {
+        'subscribers' => $subscriberActions,
-          expect($options)->contains('send_confirmation_email');
-          expect($options['send_confirmation_email'])->equals(true);
-        }),
-        'requiredCustomFieldValidator' => Stub::makeEmpty(RequiredCustomFieldValidator::class, ['validate']),
       ]
     );
     $subscriber = [

mailpoet/tests/integration/API/MP/SegmentsTest.php

@@ -8,7 +8,6 @@ use MailPoet\API\MP\v1\Segments;
 use MailPoet\API\MP\v1\Subscribers;
 use MailPoet\Config\Changelog;
 use MailPoet\Entities\SegmentEntity;
-use MailPoet\Subscribers\RequiredCustomFieldValidator;
 use MailPoet\Test\DataFactories\Segment as SegmentFactory;
 
 class SegmentsTest extends \MailPoetTest {
@@ -90,7 +89,6 @@ class SegmentsTest extends \MailPoetTest {
 
   private function getApi(): API {
     return new API(
-      $this->makeEmpty(RequiredCustomFieldValidator::class),
       $this->diContainer->get(CustomFields::class),
       $this->diContainer->get(Segments::class),
       $this->diContainer->get(Subscribers::class),

mailpoet/tests/integration/API/MP/SubscribersTest.php

@@ -19,6 +19,7 @@ use MailPoet\Settings\SettingsController;
 use MailPoet\Subscribers\ConfirmationEmailMailer;
 use MailPoet\Subscribers\NewSubscriberNotificationMailer;
 use MailPoet\Subscribers\RequiredCustomFieldValidator;
+use MailPoet\Subscribers\SubscriberSaveController;
 use MailPoet\Subscribers\SubscriberSegmentRepository;
 use MailPoet\Subscribers\SubscribersRepository;
 use MailPoet\Test\DataFactories\Subscriber as SubscriberFactory;
@@ -49,9 +50,11 @@ class SubscribersTest extends \MailPoetTest {
       SettingsController::getInstance(),
       $this->diContainer->get(SubscriberSegmentRepository::class),
       $this->diContainer->get(SubscribersRepository::class),
+      $this->diContainer->get(SubscriberSaveController::class),
       $this->diContainer->get(SubscribersResponseBuilder::class),
       Stub::makeEmpty(WelcomeScheduler::class),
       $this->diContainer->get(FeaturesController::class),
+      $this->diContainer->get(RequiredCustomFieldValidator::class),
       $this->diContainer->get(WPFunctions::class)
     );
   }
@@ -61,7 +64,6 @@ class SubscribersTest extends \MailPoetTest {
       $subscriberActions = $this->getSubscribers();
     }
     return new API(
-      $this->makeEmpty(RequiredCustomFieldValidator::class),
       $this->diContainer->get(CustomFields::class),
       $this->diContainer->get(Segments::class),
       $subscriberActions,