Refactor help endpoint

[MAILPOET-5755]

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

@@ -1,9 +1,10 @@
-<?php // phpcs:ignore SlevomatCodingStandard.TypeHints.DeclareStrictTypes.DeclareStrictTypesMissing
+<?php declare(strict_types = 1);
 
 namespace MailPoet\API\JSON\v1;
 
 use MailPoet\API\JSON\Endpoint as APIEndpoint;
 use MailPoet\API\JSON\Error as APIError;
+use MailPoet\API\JSON\Response;
 use MailPoet\Config\AccessControl;
 use MailPoet\Entities\ScheduledTaskEntity;
 use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
@@ -22,47 +23,46 @@ class Help extends APIEndpoint {
     $this->scheduledTasksRepository = $scheduledTasksRepository;
   }
 
-  public function cancelTask($data) {
+  public function cancelTask($data): Response {
-    $task = $this->validateAndFindTask($data);
-    if (!($task instanceof ScheduledTaskEntity)) {
-      return $task;
-    }
     try {
+      $this->validateTaskId($data);
+
+      $task = $this->scheduledTasksRepository->findOneById($data['id']);
+      if (!$task instanceof ScheduledTaskEntity) {
+        return $this->errorResponse([
+          APIError::NOT_FOUND => __('Task not found.', 'mailpoet'),
+        ]);
+      }
+
       $this->scheduledTasksRepository->cancelTask($task);
       return $this->successResponse();
     } catch (\Exception $e) {
-      return $this->handleException($e->getMessage());
+      return $this->badRequest([ApiError::BAD_REQUEST => $e->getMessage()]);
     }
   }
 
-  public function rescheduleTask($data) {
+  public function rescheduleTask($data): Response {
-    $task = $this->validateAndFindTask($data);
-    if (!($task instanceof ScheduledTaskEntity)) {
-      return $task;
-    }
     try {
+      $this->validateTaskId($data);
+
+      $task = $this->scheduledTasksRepository->findOneById($data['id']);
+      if (!$task instanceof ScheduledTaskEntity) {
+        return $this->errorResponse([
+          APIError::NOT_FOUND => __('Task not found.', 'mailpoet'),
+        ]);
+      }
+
       $this->scheduledTasksRepository->rescheduleTask($task);
       return $this->successResponse();
     } catch (\Exception $e) {
-      return $this->handleException($e->getMessage());
+      return $this->badRequest([ApiError::BAD_REQUEST => $e->getMessage()]);
     }
   }
 
-  private function validateAndFindTask($data) {
+  private function validateTaskId($data): void {
-    if (!isset($data['id'])) {
+    $isValid = isset($data['id']) && is_numeric($data['id']);
-      return $this->handleException(__('Missing mandatory argument `id`.', 'mailpoet'));
+    if (!$isValid) {
+      throw new \Exception(__('Invalid or missing parameter `id`.', 'mailpoet'));
     }
-    $id = $data['id'];
-    $task = $this->scheduledTasksRepository->findOneById($id);
-    if (!$task) {
-      return $this->handleException(__('Task not found.', 'mailpoet'));
-    }
-    return $task;
-  }
-
-  private function handleException($message): \MailPoet\API\JSON\ErrorResponse {
-    return $this->badRequest([
-      ApiError::BAD_REQUEST => $message,
-    ]);
   }
 }

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

@@ -2,6 +2,7 @@
 
 namespace MailPoet\Test\API\JSON\v1;
 
+use MailPoet\API\JSON\ErrorResponse;
 use MailPoet\API\JSON\Response as APIResponse;
 use MailPoet\API\JSON\v1\Help;
 use MailPoet\Entities\ScheduledTaskEntity;
@@ -10,11 +11,8 @@ use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory;
 
 class HelpTest extends \MailPoetTest {
 
-  /** @var Help */
+  private Help $endpoint;
-  private $endpoint;
+  private ScheduledTasksRepository $scheduledTasksRepository;
-
-  /** @var ScheduledTasksRepository */
-  private $scheduledTasksRepository;
 
   public function _before() {
     parent::_before();
@@ -23,31 +21,36 @@ class HelpTest extends \MailPoetTest {
   }
 
   public function testItReturnsErrorWhenIdIsMissing() {
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->cancelTask([]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
     verify($response->status)->equals(400);
-    verify($response->errors[0]['message'])->equals('Missing mandatory argument `id`.');
+    verify($response->errors[0]['message'])->equals('Invalid or missing parameter `id`.');
 
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->rescheduleTask([]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
     verify($response->status)->equals(400);
-    verify($response->errors[0]['message'])->equals('Missing mandatory argument `id`.');
+    verify($response->errors[0]['message'])->equals('Invalid or missing parameter `id`.');
   }
 
   public function testItReturnsErrorWhenTaskDoesntExist() {
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->cancelTask(['id' => 99999]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
-    verify($response->status)->equals(400);
+    verify($response->status)->equals(404);
     verify($response->errors[0]['message'])->equals('Task not found.');
 
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->rescheduleTask(['id' => 99999]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
-    verify($response->status)->equals(400);
+    verify($response->status)->equals(404);
     verify($response->errors[0]['message'])->equals('Task not found.');
   }
 
   public function testItReturnsErrorWhenCancellingCompletedTask() {
     $task = (new ScheduledTaskFactory())->create('sending', ScheduledTaskEntity::STATUS_COMPLETED, new \DateTime());
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->cancelTask(['id' => $task->getId()]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
     verify($response->status)->equals(400);
@@ -56,6 +59,7 @@ class HelpTest extends \MailPoetTest {
 
   public function testItReturnsErrorWhenReschedulingCompletedTask() {
     $task = (new ScheduledTaskFactory())->create('sending', ScheduledTaskEntity::STATUS_COMPLETED, new \DateTime());
+    /** @var ErrorResponse $response */
     $response = $this->endpoint->rescheduleTask(['id' => $task->getId()]);
     verify($response)->instanceOf('\MailPoet\API\JSON\ErrorResponse');
     verify($response->status)->equals(400);