diff --git a/mailpoet/lib/API/JSON/v1/Newsletters.php b/mailpoet/lib/API/JSON/v1/Newsletters.php index ae136e1c26..04dd502e62 100644 --- a/mailpoet/lib/API/JSON/v1/Newsletters.php +++ b/mailpoet/lib/API/JSON/v1/Newsletters.php @@ -17,12 +17,12 @@ use MailPoet\Listing; use MailPoet\Newsletter\Listing\NewsletterListingRepository; use MailPoet\Newsletter\NewsletterSaveController; use MailPoet\Newsletter\NewslettersRepository; +use MailPoet\Newsletter\NewsletterValidator; use MailPoet\Newsletter\Preview\SendPreviewController; use MailPoet\Newsletter\Preview\SendPreviewException; use MailPoet\Newsletter\Scheduler\PostNotificationScheduler; use MailPoet\Newsletter\Scheduler\Scheduler; use MailPoet\Newsletter\Url as NewsletterUrl; -use MailPoet\Newsletter\Validator; use MailPoet\Settings\SettingsController; use MailPoet\UnexpectedValueException; use MailPoet\Util\License\Features\Subscribers as SubscribersFeature; @@ -76,11 +76,8 @@ class Newsletters extends APIEndpoint { /** @var NewsletterUrl */ private $newsletterUrl; - /** @var TrackingConfig */ - private $trackingConfig; - - /** @var Validator */ - private $validator; + /** @var NewsletterValidator */ + private $newsletterValidator; /** @var Scheduler */ private $scheduler; @@ -100,7 +97,7 @@ class Newsletters extends APIEndpoint { NewsletterSaveController $newsletterSaveController, NewsletterUrl $newsletterUrl, Scheduler $scheduler, - Validator $validator + NewsletterValidator $newsletterValidator ) { $this->listingHandler = $listingHandler; $this->wp = $wp; @@ -116,7 +113,7 @@ class Newsletters extends APIEndpoint { $this->newsletterSaveController = $newsletterSaveController; $this->newsletterUrl = $newsletterUrl; $this->scheduler = $scheduler; - $this->validator = $validator; + $this->newsletterValidator = $newsletterValidator; } public function get($data = []) { @@ -188,7 +185,7 @@ class Newsletters extends APIEndpoint { } if ($status === NewsletterEntity::STATUS_ACTIVE) { - $validationError = $this->validator->validate($newsletter); + $validationError = $this->newsletterValidator->validate($newsletter); if ($validationError !== null) { return $this->errorResponse([APIError::FORBIDDEN => $validationError], [], Response::STATUS_FORBIDDEN); } diff --git a/mailpoet/lib/API/JSON/v1/SendingQueue.php b/mailpoet/lib/API/JSON/v1/SendingQueue.php index ca4c9df623..c61810da8c 100644 --- a/mailpoet/lib/API/JSON/v1/SendingQueue.php +++ b/mailpoet/lib/API/JSON/v1/SendingQueue.php @@ -14,10 +14,10 @@ use MailPoet\Mailer\MailerFactory; use MailPoet\Models\Newsletter; use MailPoet\Models\SendingQueue as SendingQueueModel; use MailPoet\Newsletter\NewslettersRepository; +use MailPoet\Newsletter\NewsletterValidator; use MailPoet\Newsletter\Scheduler\Scheduler; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; -use MailPoet\Newsletter\Validator; use MailPoet\Segments\SubscribersFinder; use MailPoet\Tasks\Sending as SendingTask; use MailPoet\Util\License\Features\Subscribers as SubscribersFeature; @@ -45,12 +45,12 @@ class SendingQueue extends APIEndpoint { /** @var MailerFactory */ private $mailerFactory; + /** @var NewsletterValidator */ + private $newsletterValidator; + /** @var Scheduler */ private $scheduler; - /** @var Validator */ - private $validator; - public function __construct( SubscribersFeature $subscribersFeature, NewslettersRepository $newsletterRepository, @@ -59,7 +59,7 @@ class SendingQueue extends APIEndpoint { ScheduledTasksRepository $scheduledTasksRepository, MailerFactory $mailerFactory, Scheduler $scheduler, - Validator $validator + NewsletterValidator $newsletterValidator ) { $this->subscribersFeature = $subscribersFeature; $this->subscribersFinder = $subscribersFinder; @@ -68,7 +68,7 @@ class SendingQueue extends APIEndpoint { $this->scheduledTasksRepository = $scheduledTasksRepository; $this->mailerFactory = $mailerFactory; $this->scheduler = $scheduler; - $this->validator = $validator; + $this->newsletterValidator = $newsletterValidator; } public function add($data = []) { @@ -97,7 +97,7 @@ class SendingQueue extends APIEndpoint { ]); } - $validationError = $this->validator->validate($newsletterEntity); + $validationError = $this->newsletterValidator->validate($newsletterEntity); if ($validationError) { return $this->errorResponse([ APIError::BAD_REQUEST => $validationError, diff --git a/mailpoet/lib/DI/ContainerConfigurator.php b/mailpoet/lib/DI/ContainerConfigurator.php index 8b0ebf97a3..0148247cfd 100644 --- a/mailpoet/lib/DI/ContainerConfigurator.php +++ b/mailpoet/lib/DI/ContainerConfigurator.php @@ -391,7 +391,7 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Newsletter\AutomaticEmailsRepository::class)->setPublic(true); $container->autowire(\MailPoet\Newsletter\NewsletterHtmlSanitizer::class)->setPublic(true); $container->autowire(\MailPoet\Newsletter\Url::class)->setPublic(true); - $container->autowire(\MailPoet\Newsletter\Validator::class)->setPublic(true); + $container->autowire(\MailPoet\Newsletter\NewsletterValidator::class)->setPublic(true); $container->autowire(\MailPoet\Newsletter\Links\Links::class)->setPublic(true); $container->autowire(\MailPoet\Newsletter\Listing\NewsletterListingRepository::class)->setPublic(true); $container->autowire(\MailPoet\Newsletter\Options\NewsletterOptionsRepository::class)->setPublic(true); diff --git a/mailpoet/lib/Newsletter/Validator.php b/mailpoet/lib/Newsletter/NewsletterValidator.php similarity index 76% rename from mailpoet/lib/Newsletter/Validator.php rename to mailpoet/lib/Newsletter/NewsletterValidator.php index f8568598ec..085803f98f 100644 --- a/mailpoet/lib/Newsletter/Validator.php +++ b/mailpoet/lib/Newsletter/NewsletterValidator.php @@ -7,7 +7,7 @@ use MailPoet\Services\Bridge; use MailPoet\Settings\TrackingConfig; use MailPoet\Validator\ValidationException; -class Validator { +class NewsletterValidator { /** @var Bridge */ private $bridge; @@ -27,7 +27,8 @@ class Validator { try { $this->validateBody($newsletterEntity); $this->validateUnsubscribeRequirements($newsletterEntity); - $this->validateReengagementRequirements($newsletterEntity); + $this->validateReEngagementRequirements($newsletterEntity); + $this->validateAutomaticLatestContentRequirements($newsletterEntity); } catch (ValidationException $exception) { return __($exception->getMessage(), 'mailpoet'); } @@ -61,7 +62,7 @@ class Validator { } } - private function validateReengagementRequirements(NewsletterEntity $newsletterEntity): void { + private function validateReEngagementRequirements(NewsletterEntity $newsletterEntity): void { if ($newsletterEntity->getType() !== NewsletterEntity::TYPE_RE_ENGAGEMENT) { return; } @@ -74,4 +75,17 @@ class Validator { throw new ValidationException('Re-engagement emails are disabled because open and click tracking is disabled in MailPoet → Settings → Advanced.'); } } + + private function validateAutomaticLatestContentRequirements(NewsletterEntity $newsletterEntity) { + if ($newsletterEntity->getType() !== NewsletterEntity::TYPE_NOTIFICATION) { + return; + } + $content = $newsletterEntity->getContent(); + if ( + strpos($content, '"type":"automatedLatestContent"') === false && + strpos($content, '"type":"automatedLatestContentLayout"') === false + ) { + throw new ValidationException('Please add an “Automatic Latest Content” widget to the email from the right sidebar.'); + } + } } diff --git a/mailpoet/tests/DataFactories/Newsletter.php b/mailpoet/tests/DataFactories/Newsletter.php index 072840f047..f0884b493c 100644 --- a/mailpoet/tests/DataFactories/Newsletter.php +++ b/mailpoet/tests/DataFactories/Newsletter.php @@ -2,6 +2,7 @@ namespace MailPoet\Test\DataFactories; +use Codeception\Util\Fixtures; use MailPoet\DI\ContainerWrapper; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\NewsletterOptionEntity; @@ -12,6 +13,7 @@ use MailPoet\Entities\ScheduledTaskSubscriberEntity; use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SendingQueueEntity; use MailPoet\Entities\SubscriberEntity; +use MailPoet\Util\Security; use MailPoetVendor\Carbon\Carbon; use MailPoetVendor\Doctrine\ORM\EntityManager; @@ -57,6 +59,10 @@ class Newsletter { return $this; } + public function withDefaultBody() { + return $this->withBody(json_decode(Fixtures::get('newsletter_body_template'), true)); + } + /** * @return Newsletter */ @@ -140,6 +146,11 @@ class Newsletter { return $this; } + public function withReengagementType() { + $this->data['type'] = NewsletterEntity::TYPE_RE_ENGAGEMENT; + return $this; + } + /** * @return Newsletter */ @@ -332,6 +343,7 @@ class Newsletter { $newsletter->setSenderAddress('john.doe@example.com'); $newsletter->setSenderName('John Doe'); } + $newsletter->setHash(Security::generateHash()); if (isset($this->data['parent'])) $newsletter->setParent($this->data['parent']); if (isset($this->data['deleted_at'])) $newsletter->setDeletedAt($this->data['deleted_at']); return $newsletter; diff --git a/mailpoet/tests/integration/API/JSON/v1/NewslettersTest.php b/mailpoet/tests/integration/API/JSON/v1/NewslettersTest.php index 53dab0a62d..956ec41752 100644 --- a/mailpoet/tests/integration/API/JSON/v1/NewslettersTest.php +++ b/mailpoet/tests/integration/API/JSON/v1/NewslettersTest.php @@ -3,7 +3,6 @@ namespace MailPoet\Test\API\JSON\v1; use Codeception\Stub\Expected; -use Codeception\Util\Fixtures; use Codeception\Util\Stub; use Helper\WordPressHooks as WPHooksHelper; use MailPoet\API\JSON\Response as APIResponse; @@ -26,6 +25,7 @@ use MailPoet\Models\SendingQueue; use MailPoet\Newsletter\Listing\NewsletterListingRepository; use MailPoet\Newsletter\NewsletterSaveController; use MailPoet\Newsletter\NewslettersRepository; +use MailPoet\Newsletter\NewsletterValidator; use MailPoet\Newsletter\Options\NewsletterOptionFieldsRepository; use MailPoet\Newsletter\Options\NewsletterOptionsRepository; use MailPoet\Newsletter\Preview\SendPreviewController; @@ -36,13 +36,12 @@ use MailPoet\Newsletter\Segment\NewsletterSegmentRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; use MailPoet\Newsletter\Statistics\NewsletterStatisticsRepository; use MailPoet\Newsletter\Url; -use MailPoet\Newsletter\Validator; use MailPoet\Router\Router; use MailPoet\Segments\SegmentsRepository; use MailPoet\Settings\SettingsController; use MailPoet\Tasks\Sending as SendingTask; +use MailPoet\Test\DataFactories\Newsletter; use MailPoet\Util\License\Features\Subscribers as SubscribersFeature; -use MailPoet\Util\Security; use MailPoet\WooCommerce\Helper as WCHelper; use MailPoet\WP\Emoji; use MailPoet\WP\Functions as WPFunctions; @@ -113,8 +112,8 @@ class NewslettersTest extends \MailPoetTest { ), ] ); - $this->newsletter = $this->createNewsletter('My Standard Newsletter', NewsletterEntity::TYPE_STANDARD); - $this->postNotification = $this->createNewsletter('My Post Notification', NewsletterEntity::TYPE_NOTIFICATION); + $this->newsletter = (new Newsletter())->withDefaultBody()->withSubject('My Standard Newsletter')->create(); + $this->postNotification = (new Newsletter())->withPostNotificationsType()->withSubject('My Post Notification')->loadBodyFrom('newsletterWithALC.json')->create(); $this->createNewsletterOptionField(NewsletterOptionFieldEntity::NAME_IS_SCHEDULED, NewsletterEntity::TYPE_STANDARD); $this->createNewsletterOptionField(NewsletterOptionFieldEntity::NAME_SCHEDULED_AT, NewsletterEntity::TYPE_STANDARD); @@ -129,7 +128,7 @@ class NewslettersTest extends \MailPoetTest { if (!$sentAt) { continue; } - $sentNewsletters[$i] = $this->createNewsletter("Sent newsletter {$i}", NewsletterEntity::TYPE_STANDARD); + $sentNewsletters[$i] = (new Newsletter())->withSubject("Sent newsletter {$i}")->create(); $sentNewsletters[$i]->setSentAt($sentAt); } $this->newsletterRepository->flush(); @@ -695,21 +694,10 @@ class NewslettersTest extends \MailPoetTest { $this->diContainer->get(NewsletterSaveController::class), $this->diContainer->get(Url::class), $this->scheduler, - $this->diContainer->get(Validator::class) + $this->diContainer->get(NewsletterValidator::class) ); } - private function createNewsletter(string $subject, string $type): NewsletterEntity { - $newsletter = new NewsletterEntity(); - $newsletter->setSubject($subject); - $newsletter->setBody((array)json_decode(Fixtures::get('newsletter_body_template'), true)); - $newsletter->setType($type); - $newsletter->setHash(Security::generateHash()); - $this->newsletterRepository->persist($newsletter); - $this->newsletterRepository->flush(); - return $newsletter; - } - private function createNewsletterOptionField(string $name, string $type): NewsletterOptionFieldEntity { $optionField = new NewsletterOptionFieldEntity(); $optionField->setName($name); diff --git a/mailpoet/tests/integration/API/JSON/v1/SendingQueueTest.php b/mailpoet/tests/integration/API/JSON/v1/SendingQueueTest.php index 78567128d4..aec022aa52 100644 --- a/mailpoet/tests/integration/API/JSON/v1/SendingQueueTest.php +++ b/mailpoet/tests/integration/API/JSON/v1/SendingQueueTest.php @@ -13,12 +13,12 @@ use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\SendingQueueEntity; use MailPoet\Mailer\MailerFactory; use MailPoet\Newsletter\NewslettersRepository; +use MailPoet\Newsletter\NewsletterValidator; use MailPoet\Newsletter\Options\NewsletterOptionFieldsRepository; use MailPoet\Newsletter\Options\NewsletterOptionsRepository; use MailPoet\Newsletter\Scheduler\Scheduler; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; -use MailPoet\Newsletter\Validator; use MailPoet\Segments\SubscribersFinder; use MailPoet\Services\Bridge; use MailPoet\Settings\SettingsController; @@ -86,7 +86,7 @@ class SendingQueueTest extends \MailPoetTest { $this->diContainer->get(ScheduledTasksRepository::class), $this->diContainer->get(MailerFactory::class), $this->diContainer->get(Scheduler::class), - $this->diContainer->get(Validator::class) + $this->diContainer->get(NewsletterValidator::class) ); $res = $sendingQueue->add(['newsletter_id' => $this->newsletter->getId()]); expect($res->status)->equals(APIResponse::STATUS_FORBIDDEN); @@ -169,7 +169,7 @@ class SendingQueueTest extends \MailPoetTest { $this->diContainer->get(ScheduledTasksRepository::class), $this->diContainer->get(MailerFactory::class), $this->diContainer->get(Scheduler::class), - new Validator(Stub::make(Bridge::class, [ + new NewsletterValidator(Stub::make(Bridge::class, [ 'isMailpoetSendingServiceEnabled' => true, ]), $this->diContainer->get(TrackingConfig::class)) ); diff --git a/mailpoet/tests/integration/Newsletter/NewsletterValidatorTest.php b/mailpoet/tests/integration/Newsletter/NewsletterValidatorTest.php new file mode 100644 index 0000000000..392980f75e --- /dev/null +++ b/mailpoet/tests/integration/Newsletter/NewsletterValidatorTest.php @@ -0,0 +1,109 @@ +newsletterValidator = $this->diContainer->get(NewsletterValidator::class); + } + + public function testUnsubscribeFooterIsNotRequiredIfNotUsingMSS() { + $newsletter = (new Newsletter())->loadBodyFrom('newsletterWithTextNoFooter.json')->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->null(); + } + + public function testUnsubscribeFooterRequiredIfUsingMSS() { + $newsletter = (new Newsletter())->loadBodyFrom('newsletterWithTextNoFooter.json')->create(); + $bridge = Stub::make(Bridge::class, ['isMailpoetSendingServiceEnabled' => true]); + $validator = $this->getServiceWithOverrides(NewsletterValidator::class, ['bridge' => $bridge]); + $validationError = $validator->validate($newsletter); + expect($validationError)->equals('All emails must include an "Unsubscribe" link. Add a footer widget to your email to continue.'); + } + + public function testItRequiresBodyContent() { + $newsletter = (new Newsletter())->withBody('')->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->equals('Poet, please add prose to your masterpiece before you send it to your followers.'); + } + + public function testItRequiresContentBlocks() { + $newsletter = (new Newsletter())->withBody(['content' => ['type' => 'container', 'columnLayout' => false, 'orientation' => 'vertical', 'blocks' => []]])->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->equals('Poet, please add prose to your masterpiece before you send it to your followers.'); + } + + public function testItIsValidWithAContentBlock() { + $newsletter = (new Newsletter())->withBody(['content' => ['type' => 'container', 'columnLayout' => false, 'orientation' => 'vertical', 'blocks' => [ + [ + 'type' => 'text', + 'text' => 'Some text' + ] + ]]])->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->null(); + } + + + public function testItRequiresReengagementShortcodes() { + $newsletter = (new Newsletter())->withReengagementType()->withDefaultBody()->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->equals('A re-engagement email must include a link with [link:subscription_re_engage_url] shortcode.'); + } + + public function testReengagementNewsletterIsValidWithRequiredShortcode() { + $newsletter = (new Newsletter())->withReengagementType()->withBody([ + 'content' => [ + 'blocks' => [ + [ + 'type' => 'text', + 'text' => '[link:subscription_re_engage_url]', + ] + ] + ] + ])->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->null(); + } + + public function testItRequiresTrackingForReengagementEmails() { + $newsletter = (new Newsletter())->withReengagementType()->withBody([ + 'content' => [ + 'blocks' => [ + [ + 'type' => 'text', + 'text' => '[link:subscription_re_engage_url]', + ] + ] + ] + ])->create(); + $validator = $this->getServiceWithOverrides(NewsletterValidator::class, [ + 'trackingConfig' => Stub::make(TrackingConfig::class, ['isEmailTrackingEnabled' => false]) + ]); + $validationError = $validator->validate($newsletter); + expect($validationError)->equals('Re-engagement emails are disabled because open and click tracking is disabled in MailPoet → Settings → Advanced.'); + } + + public function testAlcEmailFailsValidationWithoutAlcBlock() { + $newsletter = (new Newsletter())->withDefaultBody()->withPostNotificationsType()->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->equals('Please add an “Automatic Latest Content” widget to the email from the right sidebar.'); + } + + public function testAlcEmailPassesWithAlcBlock() { + $newsletter = (new Newsletter())->loadBodyFrom('newsletterWithALC.json')->withPostNotificationsType()->create(); + $validationError = $this->newsletterValidator->validate($newsletter); + expect($validationError)->null(); + } + +} diff --git a/mailpoet/tests/integration/_bootstrap.php b/mailpoet/tests/integration/_bootstrap.php index c8a66a8124..148a2112dd 100644 --- a/mailpoet/tests/integration/_bootstrap.php +++ b/mailpoet/tests/integration/_bootstrap.php @@ -183,6 +183,29 @@ abstract class MailPoetTest extends \Codeception\TestCase\Test { // phpcs:ignore return $method->invokeArgs($object, $parameters); } + /** + * Retrieve a clone of a DI service with specific private/protected properties replaced + * + * @template T of object + * @param class-string $id + * @param array $overrides + * string = protected/private property name + * Object = replacement for that property + * @return T + */ + public function getServiceWithOverrides(string $id, array $overrides) { + $instance = $this->diContainer->get($id); + $clone = clone $instance; + $reflection = new \ReflectionClass($clone); + foreach ($overrides as $propertyName => $override) { + $property = $reflection->getProperty($propertyName); + $property->setAccessible(true); + $property->setValue($clone, $override); + $property->setAccessible(false); + } + return $clone; + } + public static function markTestSkipped(string $message = ''): void { $branchName = getenv('CIRCLE_BRANCH'); if ($branchName === 'master' || $branchName === 'release') {