Refactor api for creating/updating dynamic segments

[MAILPOET-3177]

lib/API/JSON/v1/DynamicSegments.php

@@ -2,22 +2,20 @@
 
 namespace MailPoet\API\JSON\v1;
 
+use InvalidArgumentException;
 use MailPoet\API\JSON\Endpoint as APIEndpoint;
 use MailPoet\API\JSON\Error;
 use MailPoet\API\JSON\Response;
 use MailPoet\API\JSON\ResponseBuilders\DynamicSegmentsResponseBuilder;
 use MailPoet\Config\AccessControl;
-use MailPoet\DynamicSegments\Exceptions\ErrorSavingException;
-use MailPoet\DynamicSegments\Exceptions\InvalidSegmentTypeException;
 use MailPoet\DynamicSegments\Mappers\DBMapper;
-use MailPoet\DynamicSegments\Mappers\FormDataMapper;
 use MailPoet\DynamicSegments\Persistence\Loading\SingleSegmentLoader;
-use MailPoet\DynamicSegments\Persistence\Saver;
 use MailPoet\Entities\SegmentEntity;
 use MailPoet\Listing\BulkActionController;
 use MailPoet\Listing\Handler;
-use MailPoet\Models\Model;
 use MailPoet\Segments\DynamicSegments\DynamicSegmentsListingRepository;
+use MailPoet\Segments\DynamicSegments\Exceptions\InvalidFilterException;
+use MailPoet\Segments\DynamicSegments\SegmentSaveController;
 use MailPoet\Segments\SegmentsRepository;
 use MailPoet\WP\Functions as WPFunctions;
 
@@ -27,12 +25,6 @@ class DynamicSegments extends APIEndpoint {
     'global' => AccessControl::PERMISSION_MANAGE_SEGMENTS,
   ];
 
-  /** @var FormDataMapper */
-  private $mapper;
-
-  /** @var Saver */
-  private $saver;
-
   /** @var SingleSegmentLoader */
   private $dynamicSegmentsLoader;
 
@@ -51,24 +43,25 @@ class DynamicSegments extends APIEndpoint {
   /** @var DynamicSegmentsResponseBuilder */
   private $segmentsResponseBuilder;
 
+  /** @var SegmentSaveController */
+  private $saveController;
+
   public function __construct(
     BulkActionController $bulkAction,
     Handler $handler,
     DynamicSegmentsListingRepository $dynamicSegmentsListingRepository,
     DynamicSegmentsResponseBuilder $segmentsResponseBuilder,
     SegmentsRepository $segmentsRepository,
-    $mapper = null,
+    SegmentSaveController $saveController,
-    $saver = null,
     $dynamicSegmentsLoader = null
   ) {
     $this->bulkAction = $bulkAction;
     $this->listingHandler = $handler;
-    $this->mapper = $mapper ?: new FormDataMapper();
-    $this->saver = $saver ?: new Saver();
     $this->dynamicSegmentsLoader = $dynamicSegmentsLoader ?: new SingleSegmentLoader(new DBMapper());
     $this->dynamicSegmentsListingRepository = $dynamicSegmentsListingRepository;
     $this->segmentsResponseBuilder = $segmentsResponseBuilder;
     $this->segmentsRepository = $segmentsRepository;
+    $this->saveController = $saveController;
   }
 
   public function get($data = []) {
@@ -92,38 +85,34 @@ class DynamicSegments extends APIEndpoint {
 
   public function save($data) {
     try {
-      $dynamicSegment = $this->mapper->mapDataToDB($data);
+      $segment = $this->saveController->save($data);
-      $this->saver->save($dynamicSegment);
+      return $this->successResponse($this->segmentsResponseBuilder->build($segment));
-
+    } catch (InvalidFilterException $e) {
-      return $this->successResponse($data);
-    } catch (InvalidSegmentTypeException $e) {
       return $this->errorResponse([
         Error::BAD_REQUEST => $this->getErrorString($e),
       ], [], Response::STATUS_BAD_REQUEST);
-    } catch (ErrorSavingException $e) {
+    } catch (InvalidArgumentException $e) {
-      $statusCode = Response::STATUS_UNKNOWN;
+      return $this->badRequest([
-      if ($e->getCode() === Model::DUPLICATE_RECORD) {
+        Error::BAD_REQUEST  => __('Another record already exists. Please specify a different "name".', 'mailpoet'),
-        $statusCode = Response::STATUS_CONFLICT;
+      ]);
-      }
-      return $this->errorResponse([$statusCode => $e->getMessage()], [], $statusCode);
     }
   }
 
-  private function getErrorString(InvalidSegmentTypeException $e) {
+  private function getErrorString(InvalidFilterException $e) {
     switch ($e->getCode()) {
-      case InvalidSegmentTypeException::MISSING_TYPE:
+      case InvalidFilterException::MISSING_TYPE:
         return WPFunctions::get()->__('Segment type is missing.', 'mailpoet');
-      case InvalidSegmentTypeException::INVALID_TYPE:
+      case InvalidFilterException::INVALID_TYPE:
         return WPFunctions::get()->__('Segment type is unknown.', 'mailpoet');
-      case InvalidSegmentTypeException::MISSING_ROLE:
+      case InvalidFilterException::MISSING_ROLE:
         return WPFunctions::get()->__('Please select user role.', 'mailpoet');
-      case InvalidSegmentTypeException::MISSING_ACTION:
+      case InvalidFilterException::MISSING_ACTION:
         return WPFunctions::get()->__('Please select email action.', 'mailpoet');
-      case InvalidSegmentTypeException::MISSING_NEWSLETTER_ID:
+      case InvalidFilterException::MISSING_NEWSLETTER_ID:
         return WPFunctions::get()->__('Please select an email.', 'mailpoet');
-      case InvalidSegmentTypeException::MISSING_PRODUCT_ID:
+      case InvalidFilterException::MISSING_PRODUCT_ID:
         return WPFunctions::get()->__('Please select category.', 'mailpoet');
-      case InvalidSegmentTypeException::MISSING_CATEGORY_ID:
+      case InvalidFilterException::MISSING_CATEGORY_ID:
         return WPFunctions::get()->__('Please select product.', 'mailpoet');
       default:
         return WPFunctions::get()->__('An error occurred while saving data.', 'mailpoet');

lib/DI/ContainerConfigurator.php

@@ -263,6 +263,8 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Segments\DynamicSegments\Filters\UserRole::class)->setPublic(true);
     $container->autowire(\MailPoet\Segments\DynamicSegments\Filters\WooCommerceProduct::class)->setPublic(true);
     $container->autowire(\MailPoet\Segments\DynamicSegments\Filters\WooCommerceCategory::class)->setPublic(true);
+    $container->autowire(\MailPoet\Segments\DynamicSegments\SegmentSaveController::class)->setPublic(true);
+    $container->autowire(\MailPoet\Segments\DynamicSegments\FilterDataMapper::class)->setPublic(true);
     // Services
     $container->autowire(\MailPoet\Services\Bridge::class)->setPublic(true);
     $container->autowire(\MailPoet\Services\AuthorizedEmailsController::class)->setPublic(true);

lib/Entities/DynamicSegmentFilterEntity.php

@@ -53,4 +53,8 @@ class DynamicSegmentFilterEntity {
   public function setSegment(SegmentEntity $segment) {
     $this->segment = $segment;
   }
+
+  public function setFilterData(DynamicSegmentFilterData $filterData) {
+    $this->filterData = $filterData;
+  }
 }

lib/Segments/DynamicSegments/SegmentSaveController.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace MailPoet\Segments\DynamicSegments;
+
+use InvalidArgumentException;
+use MailPoet\Entities\SegmentEntity;
+use MailPoet\Segments\SegmentsRepository;
+
+class SegmentSaveController {
+  /** @var SegmentsRepository */
+  private $segmentsRepository;
+
+  /** @var FilterDataMapper */
+  private $filterDataMapper;
+
+  public function __construct(
+    SegmentsRepository $segmentsRepository,
+    FilterDataMapper $filterDataMapper
+  ) {
+    $this->segmentsRepository = $segmentsRepository;
+    $this->filterDataMapper = $filterDataMapper;
+  }
+
+  public function save(array $data = []): SegmentEntity {
+    $id = isset($data['id']) ? (int)$data['id'] : null;
+    $name = $data['name'] ?? '';
+    $description = $data['description'] ?? '';
+    $filterData = $this->filterDataMapper->map($data);
+
+    $this->checkSegmentUniqueName($name, $id);
+
+    return $this->segmentsRepository->createOrUpdate($name, $description, SegmentEntity::TYPE_DYNAMIC, $filterData, $id);
+  }
+
+  private function checkSegmentUniqueName(string $name, ?int $id): void {
+    if (!$this->segmentsRepository->isNameUnique($name, $id)) {
+      throw new InvalidArgumentException("Segment with name: '{$name}' already exists.");
+    }
+  }
+}

lib/Segments/SegmentSaveController.php

@@ -29,7 +29,7 @@ class SegmentSaveController {
 
     $this->checkSegmentUniqueName($name, $id);
 
-    return $this->segmentsRepository->createOrUpdate($name, $description, $id);
+    return $this->segmentsRepository->createOrUpdate($name, $description, SegmentEntity::TYPE_DEFAULT, null, $id);
   }
 
   public function duplicate(SegmentEntity $segmentEntity): SegmentEntity {

lib/Segments/SegmentsRepository.php

@@ -4,6 +4,8 @@ namespace MailPoet\Segments;
 
 use DateTime;
 use MailPoet\Doctrine\Repository;
+use MailPoet\Entities\DynamicSegmentFilterData;
+use MailPoet\Entities\DynamicSegmentFilterEntity;
 use MailPoet\Entities\SegmentEntity;
 use MailPoet\Entities\SubscriberSegmentEntity;
 use MailPoet\NotFoundException;
@@ -58,6 +60,8 @@ class SegmentsRepository extends Repository {
   public function createOrUpdate(
     string $name,
     string $description = '',
+    string $type = SegmentEntity::TYPE_DEFAULT,
+    ?DynamicSegmentFilterData $filterData = null,
     ?int $id = null
   ): SegmentEntity {
     if ($id) {
@@ -68,9 +72,21 @@ class SegmentsRepository extends Repository {
       $segment->setName($name);
       $segment->setDescription($description);
     } else {
-      $segment = new SegmentEntity($name, SegmentEntity::TYPE_DEFAULT, $description);
+      $segment = new SegmentEntity($name, $type, $description);
       $this->persist($segment);
     }
+
+    if ($filterData instanceof DynamicSegmentFilterData) {
+      // So far we allow only one filter
+      $filterEntity = $segment->getDynamicFilters()->first();
+      if (!$filterEntity instanceof DynamicSegmentFilterEntity) {
+        $filterEntity = new DynamicSegmentFilterEntity($segment, $filterData);
+        $segment->getDynamicFilters()->add($filterEntity);
+        $this->entityManager->persist($filterEntity);
+      } else {
+        $filterEntity->setFilterData($filterData);
+      }
+    }
     $this->flush();
     return $segment;
   }

tests/integration/API/JSON/v1/DynamicSegmentsTest.php

@@ -3,11 +3,8 @@
 namespace MailPoet\API\JSON\v1;
 
 use Codeception\Stub;
-use Codeception\Stub\Expected;
 use MailPoet\API\JSON\ResponseBuilders\DynamicSegmentsResponseBuilder;
 use MailPoet\DI\ContainerWrapper;
-use MailPoet\DynamicSegments\Exceptions\ErrorSavingException;
-use MailPoet\DynamicSegments\Exceptions\InvalidSegmentTypeException;
 use MailPoet\Entities\DynamicSegmentFilterData;
 use MailPoet\Entities\DynamicSegmentFilterEntity;
 use MailPoet\Entities\SegmentEntity;
@@ -15,8 +12,8 @@ use MailPoet\Listing\BulkActionController;
 use MailPoet\Listing\Handler;
 use MailPoet\Models\DynamicSegment;
 use MailPoet\Models\DynamicSegmentFilter;
-use MailPoet\Models\Model;
 use MailPoet\Segments\DynamicSegments\DynamicSegmentsListingRepository;
+use MailPoet\Segments\DynamicSegments\SegmentSaveController;
 use MailPoet\Segments\SegmentsRepository;
 
 class DynamicSegmentsTest extends \MailPoetTest {
@@ -37,6 +34,8 @@ class DynamicSegmentsTest extends \MailPoetTest {
   private $responseBuilder;
   /** @var SegmentsRepository */
   private $segmentsRepository;
+  /** @var SegmentSaveController */
+  private $saveController;
 
   public function _before() {
     $this->bulkAction = ContainerWrapper::getInstance()->get(BulkActionController::class);
@@ -44,6 +43,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
     $this->listingRepository = ContainerWrapper::getInstance()->get(DynamicSegmentsListingRepository::class);
     $this->responseBuilder = ContainerWrapper::getInstance()->get(DynamicSegmentsResponseBuilder::class);
     $this->segmentsRepository = ContainerWrapper::getInstance()->get(SegmentsRepository::class);
+    $this->saveController = ContainerWrapper::getInstance()->get(SegmentSaveController::class);
   }
 
   public function testGetReturnsResponse() {
@@ -54,9 +54,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
       $this->listingRepository,
       $this->responseBuilder,
       $this->segmentsRepository,
-    null,
+      $this->saveController
-      null,
-      null
     );
     $response = $endpoint->get(['id' => $segment->getId()]);
     expect($response)->isInstanceOf('\MailPoet\API\JSON\SuccessResponse');
@@ -65,59 +63,47 @@ class DynamicSegmentsTest extends \MailPoetTest {
   }
 
   public function testGetReturnsError() {
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, null, null, null);
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController);
     $response = $endpoint->get(['id' => 5]);
     expect($response)->isInstanceOf('\MailPoet\API\JSON\ErrorResponse');
     expect($response->status)->equals(self::SEGMENT_NOT_FOUND_RESPONSE_CODE);
   }
 
   public function testSaverSavesData() {
-    $mapper = Stub::makeEmpty('\MailPoet\DynamicSegments\Mappers\FormDataMapper', ['mapDataToDB' => Expected::once(function () {
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController);
-      $dynamicSegment = DynamicSegment::create();
+    $response = $endpoint->save([
-      $dynamicSegment->hydrate([
+      'name' => 'Test dynamic',
-        'name' => 'name',
+      'description' => 'description dynamic',
-        'description' => 'description',
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
-      ]);
+      'wordpressRole' => 'editor',
-      return $dynamicSegment;
+    ]);
-    })]);
-    $saver = Stub::makeEmpty('\MailPoet\DynamicSegments\Persistence\Saver', ['save' => Expected::once()]);
-
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $mapper, $saver);
-    $response = $endpoint->save([]);
     expect($response)->isInstanceOf('\MailPoet\API\JSON\SuccessResponse');
     expect($response->status)->equals(self::SUCCESS_RESPONSE_CODE);
+    expect($response->data['name'])->equals('Test dynamic');
   }
 
-  public function testSaverReturnsErrorOnInvalidData() {
+  public function testSaverReturnsErrorOnInvalidFilterData() {
-    $mapper = Stub::makeEmpty('\MailPoet\DynamicSegments\Mappers\FormDataMapper', ['mapDataToDB' => Expected::once(function () {
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController);
-      throw new InvalidSegmentTypeException();
+    $response = $endpoint->save([
-    })]);
+      'name' => 'Test dynamic',
-    $saver = Stub::makeEmpty('\MailPoet\DynamicSegments\Persistence\Saver', ['save' => Expected::never()]);
+    ]);
-
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $mapper, $saver);
-    $response = $endpoint->save([]);
     expect($response)->isInstanceOf('\MailPoet\API\JSON\ErrorResponse');
     expect($response->status)->equals(self::INVALID_DATA_RESPONSE_CODE);
+    expect($response->errors[0]['message'])->equals('Segment type is missing.');
   }
 
-  public function testSaverReturnsErrorOnSave() {
+  public function testSaverReturnsErrorOnDuplicateRecord() {
-    $mapper = Stub::makeEmpty('\MailPoet\DynamicSegments\Mappers\FormDataMapper', ['mapDataToDB' => Expected::once(function () {
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController);
-      $dynamicSegment = DynamicSegment::create();
+    $data = [
-      $dynamicSegment->hydrate([
+      'name' => 'Test dynamic',
-        'name' => 'name',
+      'description' => 'description dynamic',
-        'description' => 'description',
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
-      ]);
+      'wordpressRole' => 'editor',
-      return $dynamicSegment;
+    ];
-    })]);
+    $endpoint->save($data);
-    $saver = Stub::makeEmpty('\MailPoet\DynamicSegments\Persistence\Saver', ['save' => Expected::once(function () {
+    $response = $endpoint->save($data);
-      throw new ErrorSavingException('Error saving data', Model::DUPLICATE_RECORD);
-    })]);
-
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $mapper, $saver);
-    $response = $endpoint->save([]);
     expect($response)->isInstanceOf('\MailPoet\API\JSON\ErrorResponse');
-    expect($response->status)->equals(self::SERVER_ERROR_RESPONSE_CODE);
+    expect($response->status)->equals(self::INVALID_DATA_RESPONSE_CODE);
-    expect($response->errors[0]['message'])->equals('Error saving data');
   }
 
   public function testItCanTrashASegment() {
@@ -132,7 +118,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
       },
     ]);
 
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, null, null, $loader);
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController, $loader);
     $response = $endpoint->trash(['id' => $dynamicSegment->id]);
 
     expect($response->status)->equals(self::SUCCESS_RESPONSE_CODE);
@@ -158,7 +144,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
       },
     ]);
 
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, null, null, $loader);
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController, $loader);
     $response = $endpoint->restore(['id' => $dynamicSegment->id]);
 
     expect($response->status)->equals(self::SUCCESS_RESPONSE_CODE);
@@ -187,7 +173,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
       },
     ]);
 
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, null, null, $loader);
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController, $loader);
     $response = $endpoint->delete(['id' => $dynamicSegment->id]);
 
     expect($response->status)->equals(self::SUCCESS_RESPONSE_CODE);
@@ -218,9 +204,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
       $this->listingRepository,
       $this->responseBuilder,
       $this->segmentsRepository,
-      null,
+      $this->saveController
-      null,
-      null
     );
     $response = $endpoint->bulkAction([
       'action' => 'trash',
@@ -260,4 +244,10 @@ class DynamicSegmentsTest extends \MailPoetTest {
     $this->entityManager->flush();
     return $segment;
   }
+
+  public function _after() {
+    parent::_after();
+    $this->truncateEntity(SegmentEntity::class);
+    $this->truncateEntity(DynamicSegmentFilterEntity::class);
+  }
 }

tests/integration/Segments/DynamicSegments/SegmentSaveControllerTest.php

@@ -0,0 +1,79 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Segments\DynamicSegments;
+
+use MailPoet\Entities\DynamicSegmentFilterData;
+use MailPoet\Entities\DynamicSegmentFilterEntity;
+use MailPoet\Entities\SegmentEntity;
+use MailPoet\Segments\DynamicSegments\Exceptions\InvalidFilterException;
+
+class SegmentSaveControllerTest extends \MailPoetTest {
+  /** @var SegmentSaveController */
+  private $saveController;
+
+  public function _before() {
+    parent::_before();
+    $this->cleanup();
+    $this->saveController = $this->diContainer->get(SegmentSaveController::class);
+  }
+
+  public function testItCanSaveASegment() {
+    $segmentData = [
+      'name' => 'Test Segment',
+      'description' => 'Description',
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
+      'wordpressRole' => 'editor',
+    ];
+
+    $segment = $this->saveController->save($segmentData);
+    expect($segment->getName())->equals('Test Segment');
+    expect($segment->getDescription())->equals('Description');
+    expect($segment->getDynamicFilters()->count())->equals(1);
+    expect($segment->getType())->equals(SegmentEntity::TYPE_DYNAMIC);
+    $filter = $segment->getDynamicFilters()->first();
+    assert($filter instanceof DynamicSegmentFilterEntity);
+    expect($filter->getFilterData()->getData())->equals([
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
+      'wordpressRole' => 'editor',
+    ]);
+  }
+
+  public function testItCheckDuplicateSegment() {
+    $name = 'Test name';
+    $this->createSegment($name);
+    $segmentData = [
+      'name' => $name,
+      'description' => 'Description',
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
+      'wordpressRole' => 'editor',
+    ];
+    $this->expectException(\InvalidArgumentException::class);
+    $this->expectExceptionMessage("Segment with name: 'Test name' already exists.");
+    $this->saveController->save($segmentData);
+  }
+
+  public function testItValidatesSegmentFilterData() {
+    $name = 'Test name';
+    $this->createSegment($name);
+    $segmentData = [
+      'name' => $name,
+      'description' => 'Description',
+      'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
+      'wordpressRole' => null,
+    ];
+    $this->expectException(InvalidFilterException::class);
+    $this->saveController->save($segmentData);
+  }
+
+  private function createSegment(string $name): SegmentEntity {
+    $segment = new SegmentEntity($name, SegmentEntity::TYPE_DEFAULT, 'description');
+    $this->entityManager->persist($segment);
+    $this->entityManager->flush();
+    return $segment;
+  }
+
+  private function cleanup() {
+    $this->truncateEntity(SegmentEntity::class);
+    $this->truncateEntity(DynamicSegmentFilterEntity::class);
+  }
+}