Display proper notices for segment actions

This updates some of the segment listing messages to include the name of
 the segment to avoid confusion, and allows for the possibility that
 bulk actions will work for some segments but not all, in which case we
 need to show both success and error messages.

MAILPOET-5395

mailpoet/assets/js/src/ajax.ts

@@ -7,6 +7,13 @@ export type Response = {
   meta?: any; // eslint-disable-line @typescript-eslint/no-explicit-any
 };
 
+export interface DynamicSegmentResponse extends Response {
+  meta: {
+    count: number;
+    errors?: string[];
+  };
+}
+
 export type ErrorResponse = {
   errors: {
     error?: string;

mailpoet/assets/js/src/segments/dynamic/list/action-confirm.tsx

@@ -5,6 +5,7 @@ import { __, _n, sprintf } from '@wordpress/i18n';
 import { DynamicSegment, DynamicSegmentAction } from '../types';
 import { MailPoet } from '../../../mailpoet';
 import { storeName } from '../store';
+import { DynamicSegmentResponse } from '../../../ajax';
 
 async function bulkAction(
   action: DynamicSegmentAction,
@@ -13,51 +14,6 @@ async function bulkAction(
   if (!action) {
     return;
   }
-  let successMessage = '';
-  let errorMessage = '';
-  switch (action) {
-    case 'trash':
-      successMessage = sprintf(
-        /* translators: %d - number of segments */
-        _n(
-          'Segment moved to trash.',
-          '%d segments moved to trash.',
-          segments.length,
-          'mailpoet',
-        ),
-        segments.length,
-      );
-      errorMessage = __('Error moving segment to trash.', 'mailpoet');
-      break;
-    case 'delete':
-      successMessage = sprintf(
-        /* translators: %d - number of segments */
-        _n(
-          'Segment permanently deleted.',
-          '%d segments permanently deleted.',
-          segments.length,
-          'mailpoet',
-        ),
-        segments.length,
-      );
-      errorMessage = __('Error deleting segment.', 'mailpoet');
-      break;
-    case 'restore':
-      successMessage = sprintf(
-        /* translators: %d - number of segments */
-        _n(
-          'Segment restored.',
-          '%d segments restored.',
-          segments.length,
-          'mailpoet',
-        ),
-        segments.length,
-      );
-      errorMessage = __('Error restoring segment.', 'mailpoet');
-      break;
-    default:
-      break;
-  }
   void MailPoet.Ajax.post({
     api_version: 'v1',
     endpoint: 'dynamic_segments',
@@ -69,14 +25,86 @@ async function bulkAction(
       },
     },
   })
-    .then(() => {
+    .then((response: DynamicSegmentResponse) => {
-      void dispatch(noticesStore).createSuccessNotice(successMessage);
+      if (response.meta.errors && response.meta.errors.length > 0) {
-      void dispatch(storeName).loadDynamicSegments();
+        response.meta.errors.forEach(
+          (error: string) =>
+            void dispatch(noticesStore).createErrorNotice(error),
+        );
+      }
+      const count = response.meta.count;
+      if (count > 0) {
+        let successMessage = '';
+        switch (action) {
+          case 'trash':
+            successMessage = sprintf(
+              /* translators: %d - number of segments */
+              _n(
+                'Segment moved to trash.',
+                '%d segments moved to trash.',
+                count,
+                'mailpoet',
+              ),
+              count,
+            );
+            break;
+          case 'delete':
+            successMessage = sprintf(
+              /* translators: %d - number of segments */
+              _n(
+                'Segment permanently deleted.',
+                '%d segments permanently deleted.',
+                count,
+                'mailpoet',
+              ),
+              count,
+            );
+            break;
+          case 'restore':
+            successMessage = sprintf(
+              /* translators: %d - number of segments */
+              _n(
+                'Segment restored.',
+                '%d segments restored.',
+                count,
+                'mailpoet',
+              ),
+              count,
+            );
+            break;
+          default:
+            break;
+        }
+        void dispatch(noticesStore).createSuccessNotice(successMessage);
+        void dispatch(storeName).loadDynamicSegments();
+      }
     })
-    .fail(() => {
+    .fail((response: ErrorResponse) => {
-      void dispatch(noticesStore).createErrorNotice(errorMessage, {
+      let errorMessage = '';
-        explicitDismiss: true,
+      if (response.errors) {
-      });
+        response.errors.forEach(
+          (error) =>
+            void dispatch(noticesStore).createErrorNotice(error.message),
+          { explicitDismiss: true },
+        );
+      } else {
+        switch (action) {
+          case 'trash':
+            errorMessage = __('Error moving segment to trash.', 'mailpoet');
+            break;
+          case 'delete':
+            errorMessage = __('Error deleting segment.', 'mailpoet');
+            break;
+          case 'restore':
+            errorMessage = __('Error restoring segment.', 'mailpoet');
+            break;
+          default:
+            break;
+        }
+        void dispatch(noticesStore).createErrorNotice(errorMessage, {
+          explicitDismiss: true,
+        });
+      }
     });
 }
 

mailpoet/assets/js/src/subscribers/list.tsx

@@ -50,6 +50,7 @@ type Response = {
     count: number;
     segment: string;
     tag: string;
+    errors?: string[];
   };
 };
 

mailpoet/lib/API/JSON/v1/DynamicSegments.php

@@ -198,16 +198,9 @@ class DynamicSegments extends APIEndpoint {
       ]);
     }
 
-    $activelyUsedNewslettersSubjects = $this->newsletterSegmentRepository->getSubjectsOfActivelyUsedEmailsForSegments([$segment->getId()]);
+    $activelyUsedErrors = $this->getErrorMessagesForSegmentsUsedInActiveNewsletters([$segment->getId()]);
-    if (isset($activelyUsedNewslettersSubjects[$segment->getId()])) {
+    if (count($activelyUsedErrors) > 0) {
-      return $this->badRequest([
+      return $this->badRequest($activelyUsedErrors);
-        Error::BAD_REQUEST => str_replace(
-          '%1$s',
-          "'" . join("', '", $activelyUsedNewslettersSubjects[$segment->getId()]) . "'",
-          // translators: %1$s is a comma-seperated list of emails for which the segment is used.
-          _x('Segment cannot be deleted because it’s used for %1$s email', 'Alert shown when trying to delete segment, which is assigned to any automatic emails.', 'mailpoet')
-        ),
-      ]);
     }
 
     $this->segmentsRepository->bulkTrash([$segment->getId()], SegmentEntity::TYPE_DYNAMIC);
@@ -217,6 +210,26 @@ class DynamicSegments extends APIEndpoint {
     );
   }
 
+  public function getErrorMessagesForSegmentsUsedInActiveNewsletters(array $segmentIds): array {
+    $errors = [];
+    $activelyUsedNewslettersSubjects = $this->newsletterSegmentRepository->getSubjectsOfActivelyUsedEmailsForSegments($segmentIds);
+    foreach ($segmentIds as $segmentId) {
+      if (isset($activelyUsedNewslettersSubjects[$segmentId])) {
+        $segment = $this->getSegment(['id' => $segmentId]);
+        if ($segment) {
+          $errors[] = sprintf(
+            // translators: %1$s is the name of the segment, %2$s is a comma-seperated list of emails for which the segment is used.
+            _x('Segment \'%1$s\' cannot be deleted because it’s used for \'%2$s\' email', 'Alert shown when trying to delete segment, which is assigned to any automatic emails.', 'mailpoet'),
+            $segment->getName(),
+            join("', '", $activelyUsedNewslettersSubjects[$segmentId])
+          );
+        }
+      }
+    }
+
+    return $errors;
+  }
+
   public function restore($data = []) {
     if (!isset($data['id'])) {
       return $this->errorResponse([
@@ -275,17 +288,22 @@ class DynamicSegments extends APIEndpoint {
   public function bulkAction($data = []) {
     $definition = $this->listingHandler->getListingDefinition($data['listing']);
     $ids = $this->dynamicSegmentsListingRepository->getActionableIds($definition);
+    $meta = [];
     if ($data['action'] === 'trash') {
-      $count = $this->segmentsRepository->bulkTrash($ids, SegmentEntity::TYPE_DYNAMIC);
+      $errors = $this->getErrorMessagesForSegmentsUsedInActiveNewsletters($ids);
+      if (count($errors) > 0) {
+        $meta['errors'] = $errors;
+      }
+      $meta['count'] = $this->segmentsRepository->bulkTrash($ids, SegmentEntity::TYPE_DYNAMIC);
     } elseif ($data['action'] === 'restore') {
-      $count = $this->segmentsRepository->bulkRestore($ids, SegmentEntity::TYPE_DYNAMIC);
+      $meta['count'] = $this->segmentsRepository->bulkRestore($ids, SegmentEntity::TYPE_DYNAMIC);
     } elseif ($data['action'] === 'delete') {
-      $count = $this->segmentsRepository->bulkDelete($ids, SegmentEntity::TYPE_DYNAMIC);
+      $meta['count'] = $this->segmentsRepository->bulkDelete($ids, SegmentEntity::TYPE_DYNAMIC);
     } else {
       throw UnexpectedValueException::create()
         ->withErrors([Error::BAD_REQUEST => "Invalid bulk action '{$data['action']}' provided."]);
     }
-    return $this->successResponse(null, ['count' => $count]);
+    return $this->successResponse(null, $meta);
   }
 
   private function getSegment(array $data): ?SegmentEntity {

mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php

@@ -308,12 +308,13 @@ class ManageSegmentsCest {
     $i->amOnMailpoetPage('Segments');
     $i->waitForText($segmentTitle, 5, '[data-automation-id="mailpoet_dynamic_segment_name_' . $segment->getId() . '"]');
     $i->clickWooTableActionInsideMoreButton($segmentTitle, 'Move to trash');
-    $i->waitForText("Segment cannot be deleted because it’s used for '{$subject}' email");
+    $i->click(['xpath' => '//button[text()="Trash"]']); // confirmation modal, xpath to avoid clicking the Trash tab
+    $i->waitForText("Segment '{$segmentTitle}' cannot be deleted because it’s used for '{$subject}' email");
     $i->seeNoJSErrors();
-    $i->checkOption('[data-automation-id="listing-row-checkbox-' . $segment->getId() . '"]');
+    $i->checkWooTableCheckboxForItemName($segment->getName());
-    $i->waitForText('Move to trash');
+    $i->selectOption('Bulk Actions', 'Trash');
-    $i->click('Move to trash');
+    $i->click(['xpath' => '//button[text()="Trash"]']); // confirmation modal, xpath to avoid clicking the Trash tab
-    $i->waitForText('0 segments were moved to the trash.');
+    $i->waitForText("Segment '{$segmentTitle}' cannot be deleted because it’s used for '{$subject}' email");
   }
 
   public function createUserSegmentAndCheckCount(\AcceptanceTester $i) {

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

@@ -121,7 +121,7 @@ class DynamicSegmentsTest extends \MailPoetTest {
     $response = $this->endpoint->trash(['id' => $dynamicSegment->getId()]);
     $this->entityManager->refresh($dynamicSegment);
     verify($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
-    verify($response->errors[0]['message'])->equals("Segment cannot be deleted because it’s used for 'Subject' email");
+    verify($response->errors[0]['message'])->equals("Segment '{$dynamicSegment->getName()}' cannot be deleted because it’s used for 'Subject' email");
   }
 
   public function testItCanRestoreASegment() {