Refactor dynamic segments delete endpoint

[MAILPOET-3177]

lib/API/JSON/v1/DynamicSegments.php

@@ -8,8 +8,6 @@ use MailPoet\API\JSON\Error;
 use MailPoet\API\JSON\Response;
 use MailPoet\API\JSON\ResponseBuilders\DynamicSegmentsResponseBuilder;
 use MailPoet\Config\AccessControl;
-use MailPoet\DynamicSegments\Mappers\DBMapper;
-use MailPoet\DynamicSegments\Persistence\Loading\SingleSegmentLoader;
 use MailPoet\Entities\SegmentEntity;
 use MailPoet\Listing\BulkActionController;
 use MailPoet\Listing\Handler;
@@ -25,9 +23,6 @@ class DynamicSegments extends APIEndpoint {
     'global' => AccessControl::PERMISSION_MANAGE_SEGMENTS,
   ];
 
-  /** @var SingleSegmentLoader */
-  private $dynamicSegmentsLoader;
-
   /** @var BulkActionController */
   private $bulkAction;
 
@@ -52,12 +47,10 @@ class DynamicSegments extends APIEndpoint {
     DynamicSegmentsListingRepository $dynamicSegmentsListingRepository,
     DynamicSegmentsResponseBuilder $segmentsResponseBuilder,
     SegmentsRepository $segmentsRepository,
-    SegmentSaveController $saveController,
+    SegmentSaveController $saveController
-    $dynamicSegmentsLoader = null
   ) {
     $this->bulkAction = $bulkAction;
     $this->listingHandler = $handler;
-    $this->dynamicSegmentsLoader = $dynamicSegmentsLoader ?: new SingleSegmentLoader(new DBMapper());
     $this->dynamicSegmentsListingRepository = $dynamicSegmentsListingRepository;
     $this->segmentsResponseBuilder = $segmentsResponseBuilder;
     $this->segmentsRepository = $segmentsRepository;
@@ -162,23 +155,21 @@ class DynamicSegments extends APIEndpoint {
   }
 
   public function delete($data = []) {
-    if (isset($data['id'])) {
+    if (!isset($data['id'])) {
-      $id = (int)$data['id'];
-    } else {
       return $this->errorResponse([
         Error::BAD_REQUEST => WPFunctions::get()->__('Missing mandatory argument `id`.', 'mailpoet'),
       ]);
     }
 
-    try {
+    $segment = $this->getSegment($data);
-      $segment = $this->dynamicSegmentsLoader->load($id);
+    if ($segment === null) {
-      $segment->delete();
-      return $this->successResponse(null, ['count' => 1]);
-    } catch (\InvalidArgumentException $e) {
       return $this->errorResponse([
         Error::NOT_FOUND => WPFunctions::get()->__('This segment does not exist.', 'mailpoet'),
       ]);
     }
+
+    $this->segmentsRepository->bulkDelete([$segment->getId()], SegmentEntity::TYPE_DYNAMIC);
+    return $this->successResponse(null, ['count' => 1]);
   }
 
   public function listing($data = []) {

lib/Segments/SegmentsRepository.php

@@ -91,32 +91,40 @@ class SegmentsRepository extends Repository {
     return $segment;
   }
 
-  public function bulkDelete(array $ids) {
+  public function bulkDelete(array $ids, $type = SegmentEntity::TYPE_DEFAULT) {
     if (empty($ids)) {
       return 0;
     }
 
-    return $this->entityManager->transactional(function (EntityManager $entityManager) use ($ids) {
+    return $this->entityManager->transactional(function (EntityManager $entityManager) use ($ids, $type) {
       $subscriberSegmentTable = $entityManager->getClassMetadata(SubscriberSegmentEntity::class)->getTableName();
       $segmentTable = $entityManager->getClassMetadata(SegmentEntity::class)->getTableName();
+      $segmentFiltersTable = $entityManager->getClassMetadata(DynamicSegmentFilterEntity::class)->getTableName();
 
       $entityManager->getConnection()->executeUpdate("
          DELETE ss FROM $subscriberSegmentTable ss
          JOIN $segmentTable s ON ss.`segment_id` = s.`id`
          WHERE ss.`segment_id` IN (:ids)
-         AND s.`type` = :typeDefault
+         AND s.`type` = :type
+      ", [
+        'ids' => $ids,
+        'type' => $type,
+      ], ['ids' => Connection::PARAM_INT_ARRAY]);
+
+      $entityManager->getConnection()->executeUpdate("
+         DELETE df FROM $segmentFiltersTable df
+         WHERE df.`segment_id` IN (:ids)
       ", [
         'ids' => $ids,
-        'typeDefault' => SegmentEntity::TYPE_DEFAULT,
       ], ['ids' => Connection::PARAM_INT_ARRAY]);
 
       return $entityManager->getConnection()->executeUpdate("
          DELETE s FROM $segmentTable s
          WHERE s.`id` IN (:ids)
-         AND s.`type` = :typeDefault
+         AND s.`type` = :type
       ", [
         'ids' => $ids,
-        'typeDefault' => SegmentEntity::TYPE_DEFAULT,
+        'type' => $type,
       ], ['ids' => Connection::PARAM_INT_ARRAY]);
     });
   }

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

@@ -2,7 +2,6 @@
 
 namespace MailPoet\API\JSON\v1;
 
-use Codeception\Stub;
 use MailPoet\API\JSON\ResponseBuilders\DynamicSegmentsResponseBuilder;
 use MailPoet\DI\ContainerWrapper;
 use MailPoet\Entities\DynamicSegmentFilterData;
@@ -137,29 +136,22 @@ class DynamicSegmentsTest extends \MailPoetTest {
   }
 
   public function testItCanDeleteASegment() {
-    DynamicSegment::deleteMany();
+    $dynamicSegment = $this->createDynamicSegmentEntity('Delete test', 'description');
-    $dynamicSegment = DynamicSegment::createOrUpdate([
+    $dynamicSegmentFilter = $dynamicSegment->getDynamicFilters()->first();
-      'name' => 'Delete test',
+    assert($dynamicSegmentFilter instanceof DynamicSegmentFilterEntity);
-      'description' => 'description',
-    ]);
-    $filter = DynamicSegmentFilter::createOrUpdate([
-      'segment_id' => $dynamicSegment->id,
-    ]);
-    $loader = Stub::makeEmpty('\MailPoet\DynamicSegments\Persistence\Loading\SingleSegmentLoader', [
-      'load' => function () use($dynamicSegment) {
-        return $dynamicSegment;
-      },
-    ]);
 
-    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController, $loader);
+    $endpoint = new DynamicSegments($this->bulkAction, $this->listingHandler, $this->listingRepository, $this->responseBuilder, $this->segmentsRepository, $this->saveController);
-    $response = $endpoint->delete(['id' => $dynamicSegment->id]);
+    $response = $endpoint->delete(['id' => $dynamicSegment->getId()]);
 
     expect($response->status)->equals(self::SUCCESS_RESPONSE_CODE);
     expect($response->data)->equals(null);
     expect($response->meta['count'])->equals(1);
 
-    expect(DynamicSegment::findOne($dynamicSegment->id))->equals(false);
+    // Clear entity manager to forget all entities
-    expect(DynamicSegmentFilter::findOne($filter->id))->equals(false);
+    $this->entityManager->clear();
+
+    expect($this->entityManager->find(SegmentEntity::class, $dynamicSegment->getId()))->null();
+    expect($this->entityManager->find(DynamicSegmentFilterEntity::class, $dynamicSegmentFilter->getId()))->null();
   }
 
   public function testItCanBulkDeleteSegments() {