PR feedback: add type hints, create new methods and save on extra api request

MAILPOET-4601

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

@@ -238,25 +238,27 @@ class Services extends APIEndpoint {
       return $this->createBadRequest(__('MailPoet Sending Service is not active.', 'mailpoet'));
     }
 
-    $authorizedEmails = $this->bridge->getAuthorizedEmailAddresses();
-
-    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomains(true);
-
-    if (!$authorizedEmails && !$verifiedDomains) {
-      return $this->createBadRequest(__('No FROM email addresses are authorized.', 'mailpoet'));
-    }
-
     $fromEmail = $this->settings->get('sender.address');
     if (!$fromEmail) {
       return $this->createBadRequest(__('Sender email address is not set.', 'mailpoet'));
     }
 
-    $arrayOfItems = explode('@', $fromEmail);
+    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomainsIgnoringCache();
-    $emailDomain = array_pop($arrayOfItems);
 
-    if (!$this->isItemInArray($fromEmail, $authorizedEmails) && !$this->isItemInArray($emailDomain, $verifiedDomains)) {
+    $arrayOfItems = explode('@', trim($fromEmail));
-      // translators: %s is the email address, which is not authorized.
+    $emailDomain = strtolower(array_pop($arrayOfItems));
-      return $this->createBadRequest(sprintf(__("Sender email address '%s' is not authorized.", 'mailpoet'), $fromEmail));
+
+    if (!$this->isItemInArray($emailDomain, $verifiedDomains)) {
+      $authorizedEmails = $this->bridge->getAuthorizedEmailAddresses();
+
+      if (!$authorizedEmails) {
+        return $this->createBadRequest(__('No FROM email addresses are authorized.', 'mailpoet'));
+      }
+
+      if (!$this->isItemInArray($fromEmail, $authorizedEmails)) {
+        // translators: %s is the email address, which is not authorized.
+        return $this->createBadRequest(sprintf(__("Sender email address '%s' is not authorized.", 'mailpoet'), $fromEmail));
+      }
     }
 
     try {

mailpoet/lib/Services/AuthorizedEmailsController.php

@@ -51,7 +51,7 @@ class AuthorizedEmailsController {
 
   public function setFromEmailAddress(string $address) {
     $authorizedEmails = $this->bridge->getAuthorizedEmailAddresses() ?: [];
-    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomains(true);
+    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomainsIgnoringCache();
     $isAuthorized = $this->validateAuthorizedEmail($authorizedEmails, $address);
 
     $emailDomainIsVerified = $this->validateEmailDomainIsVerified($verifiedDomains, $address);
@@ -122,7 +122,7 @@ class AuthorizedEmailsController {
     }
     $authorizedEmails = array_map('strtolower', $authorizedEmails);
 
-    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomains(true);
+    $verifiedDomains = $this->senderDomainController->getVerifiedSenderDomainsIgnoringCache();
 
     $result = [];
     $result = $this->validateAddressesInSettings($authorizedEmails, $verifiedDomains, $result);
@@ -213,7 +213,7 @@ class AuthorizedEmailsController {
     return in_array(strtolower($email), $lowercaseAuthorizedEmails, true);
   }
 
-  private function validateEmailDomainIsVerified($verifiedDomains = [], $email = ''): bool {
+  private function validateEmailDomainIsVerified(array $verifiedDomains = [], string $email = ''): bool {
     $lowercaseVerifiedDomains = array_map('strtolower', $verifiedDomains);
     $arrayOfItems = explode('@', $email);
     $emailDomain = array_pop($arrayOfItems);

mailpoet/lib/Services/AuthorizedSenderDomainController.php

@@ -46,23 +46,27 @@ class AuthorizedSenderDomainController {
    *
    * Note: This includes both verified and unverified domains
    */
-  public function getAllSenderDomains(bool $skipCache = false): array {
+  public function getAllSenderDomains(): array {
-    if ($skipCache) {
-      $this->currentRecords = null;
-    }
     return $this->returnAllDomains($this->getAllRecords());
   }
 
+  public function getAllSenderDomainsIgnoringCache(): array {
+    $this->currentRecords = null;
+    return $this->getAllSenderDomains();
+  }
+
   /**
    * Get all Verified Sender Domains
    */
-  public function getVerifiedSenderDomains(bool $skipCache = false): array {
+  public function getVerifiedSenderDomains(): array {
-    if ($skipCache) {
-      $this->currentRecords = null;
-    }
     return $this->returnVerifiedDomains($this->getAllRecords());
   }
 
+  public function getVerifiedSenderDomainsIgnoringCache(): array {
+    $this->currentRecords = null;
+    return $this->getVerifiedSenderDomains();
+  }
+
   /**
    * Create new Sender Domain
    *

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

@@ -479,6 +479,7 @@ class ServicesTest extends \MailPoetTest {
 
   public function testCongratulatoryEmailRespondsWithErrorWhenNoEmailAuthorized() {
     $this->settings->set(Mailer::MAILER_CONFIG_SETTING_NAME, ['method' => Mailer::METHOD_MAILPOET]);
+    $this->settings->set('sender.address', 'unauthorized@email.com');
     $bridge = $this->make(Bridge::class, [
       'getAuthorizedEmailAddresses' => [],
     ]);
@@ -497,7 +498,7 @@ class ServicesTest extends \MailPoetTest {
 
     $verifiedDomains = ['email.com'];
     $senderDomainMock = $this->make(AuthorizedSenderDomainController::class, [
-      'getVerifiedSenderDomains' => Expected::once($verifiedDomains),
+      'getVerifiedSenderDomainsIgnoringCache' => $verifiedDomains,
     ]);
 
     $servicesEndpoint = $this->createServicesEndpointWithMocks([

mailpoet/tests/integration/Services/AuthorizedEmailsControllerTest.php

@@ -78,7 +78,7 @@ class AuthorizedEmailsControllerTest extends \MailPoetTest {
 
     $verifiedDomains = ['email.com'];
     $senderDomainMock = $this->make(AuthorizedSenderDomainController::class, [
-      'getVerifiedSenderDomains' => Expected::once($verifiedDomains),
+      'getVerifiedSenderDomainsIgnoringCache' => Expected::once($verifiedDomains),
     ]);
 
     $mocks = [
@@ -150,7 +150,7 @@ class AuthorizedEmailsControllerTest extends \MailPoetTest {
 
     $verifiedDomains = ['email.com'];
     $senderDomainMock = $this->make(AuthorizedSenderDomainController::class, [
-      'getVerifiedSenderDomains' => Expected::once($verifiedDomains),
+      'getVerifiedSenderDomainsIgnoringCache' => Expected::once($verifiedDomains),
     ]);
 
     $mocks = [
@@ -227,7 +227,7 @@ class AuthorizedEmailsControllerTest extends \MailPoetTest {
 
     $verifiedDomains = ['email.com'];
     $senderDomainMock = $this->make(AuthorizedSenderDomainController::class, [
-      'getVerifiedSenderDomains' => Expected::once($verifiedDomains),
+      'getVerifiedSenderDomainsIgnoringCache' => Expected::once($verifiedDomains),
     ]);
 
     $mocks = [