From dabdc905ce849583b7818751db461c81832d5496 Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Wed, 18 Oct 2017 16:09:14 +1300 Subject: [PATCH] BUG Fix enable email subclasses to use their respective templates --- src/Control/Email/Email.php | 62 ++++++-- src/View/ThemeResourceLoader.php | 4 +- tests/php/Control/Email/EmailTest.php | 138 ++++++++++++------ .../Control/Email/EmailTest/EmailSubClass.php | 11 ++ .../Tests/Email/EmailTest/EmailSubClass.ss | 14 ++ 5 files changed, 167 insertions(+), 62 deletions(-) create mode 100644 tests/php/Control/Email/EmailTest/EmailSubClass.php create mode 100644 tests/php/Control/Email/EmailTest/templates/SilverStripe/Control/Tests/Email/EmailTest/EmailSubClass.ss diff --git a/src/Control/Email/Email.php b/src/Control/Email/Email.php index c879a6af0..0f0997f92 100644 --- a/src/Control/Email/Email.php +++ b/src/Control/Email/Email.php @@ -7,7 +7,11 @@ use SilverStripe\Control\HTTP; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\View\Requirements; +use SilverStripe\View\SSViewer; +use SilverStripe\View\ThemeResourceLoader; use SilverStripe\View\ViewableData; use Swift_Message; use Swift_MimePart; @@ -58,12 +62,12 @@ class Email extends ViewableData /** * @var string The name of the HTML template to render the email with (without *.ss extension) */ - private $HTMLTemplate = self::class; + private $HTMLTemplate = null; /** * @var string The name of the plain text template to render the plain part of the email with */ - private $plainTemplate = ''; + private $plainTemplate = null; /** * @var Swift_MimePart @@ -648,7 +652,14 @@ class Email extends ViewableData */ public function getHTMLTemplate() { - return $this->HTMLTemplate; + if ($this->HTMLTemplate) { + return $this->HTMLTemplate; + } + + return ThemeResourceLoader::inst()->findTemplate( + SSViewer::get_templates_by_class(static::class, '', self::class), + SSViewer::get_themes() + ); } /** @@ -760,30 +771,49 @@ class Email extends ViewableData $this->getSwiftMessage()->detach($existingPlainPart); } unset($existingPlainPart); - if (!$this->getHTMLTemplate() && !$this->getPlainTemplate()) { + + // Respect explicitly set body + $htmlPart = $plainOnly ? null : $this->getBody(); + $plainPart = $plainOnly ? $this->getBody() : null; + + // Ensure we can at least render something + $htmlTemplate = $this->getHTMLTemplate(); + $plainTemplate = $this->getPlainTemplate(); + if (!$htmlTemplate && !$plainTemplate && !$plainPart && !$htmlPart) { return $this; } - $HTMLPart = ''; - $plainPart = ''; - if ($this->getHTMLTemplate()) { - $HTMLPart = $this->renderWith($this->getHTMLTemplate(), $this->getData()); + // Render plain part + if ($plainTemplate && !$plainPart) { + $plainPart = $this->renderWith($plainTemplate, $this->getData()); } - if ($this->getPlainTemplate()) { - $plainPart = $this->renderWith($this->getPlainTemplate(), $this->getData()); - } elseif ($HTMLPart) { - $plainPart = Convert::xml2raw($HTMLPart); + // Render HTML part, either if sending html email, or a plain part is lacking + if (!$htmlPart && $htmlTemplate && (!$plainOnly || empty($plainPart))) { + $htmlPart = $this->renderWith($htmlTemplate, $this->getData()); } - if ($HTMLPart && !$plainOnly) { - $this->setBody($HTMLPart); + // Plain part fails over to generated from html + if (!$plainPart && $htmlPart) { + /** @var DBHTMLText $htmlPartObject */ + $htmlPartObject = DBField::create_field('HTMLFragment', $htmlPart); + $plainPart = $htmlPartObject->Plain(); + } + + // Fail if no email to send + if (!$plainPart && !$htmlPart) { + return $this; + } + + // Build HTML / Plain components + if ($htmlPart && !$plainOnly) { + $this->setBody($htmlPart); $this->getSwiftMessage()->setContentType('text/html'); $this->getSwiftMessage()->setCharset('utf-8'); if ($plainPart) { $this->getSwiftMessage()->addPart($plainPart, 'text/plain', 'utf-8'); } - } elseif ($plainPart || $plainOnly) { + } else { if ($plainPart) { $this->setBody($plainPart); } @@ -812,7 +842,7 @@ class Email extends ViewableData */ public function hasPlainPart() { - if ($this->getSwiftMessage()->getContentType() == 'text/plain') { + if ($this->getSwiftMessage()->getContentType() === 'text/plain') { return true; } return (bool) $this->findPlainPart(); diff --git a/src/View/ThemeResourceLoader.php b/src/View/ThemeResourceLoader.php index 72a5d8f21..ca06e2ba1 100644 --- a/src/View/ThemeResourceLoader.php +++ b/src/View/ThemeResourceLoader.php @@ -103,7 +103,7 @@ class ThemeResourceLoader if (count($parts) > 1) { throw new InvalidArgumentException("Invalid theme identifier {$identifier}"); } - return substr($identifier, 1); + return ltrim($identifier, '/'); } // If there is no slash / colon it's a legacy theme @@ -148,7 +148,7 @@ class ThemeResourceLoader } // Join module with subpath - return $modulePath . $subpath; + return ltrim($modulePath . $subpath, '/'); } /** diff --git a/tests/php/Control/Email/EmailTest.php b/tests/php/Control/Email/EmailTest.php index 47de7d236..a36a7b496 100644 --- a/tests/php/Control/Email/EmailTest.php +++ b/tests/php/Control/Email/EmailTest.php @@ -4,10 +4,16 @@ namespace SilverStripe\Control\Tests\Email; use PHPUnit_Framework_MockObject_MockObject; use SilverStripe\Control\Email\Email; +use SilverStripe\Control\Email\Mailer; use SilverStripe\Control\Email\SwiftMailer; +use SilverStripe\Control\Tests\Email\EmailTest\EmailSubClass; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Manifest\ModuleResourceLoader; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Dev\TestMailer; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Security\Member; +use SilverStripe\View\SSViewer; use Swift_Attachment; use Swift_Mailer; use Swift_Message; @@ -84,37 +90,25 @@ class EmailTest extends SapphireTest public function testSendPlain() { - /** @var Email|PHPUnit_Framework_MockObject_MockObject $email */ - $email = $this->getMockBuilder(Email::class) - ->enableProxyingToOriginalMethods() - ->disableOriginalConstructor() - ->setConstructorArgs(array( - 'from@example.com', - 'to@example.com', - 'Test send plain', - 'Testing Email->sendPlain()', - 'cc@example.com', - 'bcc@example.com', - )) - ->getMock(); + $email = $this->makeEmailMock('Test send plain'); // email should not call render if a body is supplied - $email->expects($this->never())->method('render'); - - $email->addAttachment(__DIR__ . '/EmailTest/attachment.txt', null, 'text/plain'); + $email->expects($this->never())->method('renderWith'); $successful = $email->sendPlain(); $this->assertTrue($successful); $this->assertEmpty($email->getFailedRecipients()); - $sentMail = $this->mailer->findEmail('to@example.com'); + /** @var TestMailer $mailer */ + $mailer = Injector::inst()->get(Mailer::class); + $sentMail = $mailer->findEmail('to@example.com'); $this->assertTrue(is_array($sentMail)); $this->assertEquals('to@example.com', $sentMail['To']); $this->assertEquals('from@example.com', $sentMail['From']); $this->assertEquals('Test send plain', $sentMail['Subject']); - $this->assertEquals('Testing Email->sendPlain()', $sentMail['Content']); + $this->assertEquals('Body for Test send plain', $sentMail['Content']); $this->assertCount(1, $sentMail['AttachedFiles']); $child = reset($sentMail['AttachedFiles']); @@ -126,36 +120,25 @@ class EmailTest extends SapphireTest public function testSend() { /** @var Email|PHPUnit_Framework_MockObject_MockObject $email */ - $email = $this->getMockBuilder(Email::class) - ->enableProxyingToOriginalMethods() - ->disableOriginalConstructor() - ->setConstructorArgs(array( - 'from@example.com', - 'to@example.com', - 'Test send HTML', - 'Testing Email->send()', - 'cc@example.com', - 'bcc@example.com', - )) - ->getMock(); + $email = $this->makeEmailMock('Test send HTML'); // email should not call render if a body is supplied - $email->expects($this->never())->method('render'); - - $email->addAttachment(__DIR__ . '/EmailTest/attachment.txt', null, 'text/plain'); + $email->expects($this->never())->method('renderWith'); $successful = $email->send(); $this->assertTrue($successful); $this->assertEmpty($email->getFailedRecipients()); - $sentMail = $this->mailer->findEmail('to@example.com'); + /** @var TestMailer $mailer */ + $mailer = Injector::inst()->get(Mailer::class); + $sentMail = $mailer->findEmail('to@example.com'); $this->assertTrue(is_array($sentMail)); $this->assertEquals('to@example.com', $sentMail['To']); $this->assertEquals('from@example.com', $sentMail['From']); $this->assertEquals('Test send HTML', $sentMail['Subject']); - $this->assertEquals('Testing Email->send()', $sentMail['Content']); + $this->assertEquals('Body for Test send HTML', $sentMail['Content']); $this->assertCount(1, $sentMail['AttachedFiles']); $child = reset($sentMail['AttachedFiles']); @@ -169,12 +152,9 @@ class EmailTest extends SapphireTest /** @var Email|PHPUnit_Framework_MockObject_MockObject $email */ $email = $this->getMockBuilder(Email::class) ->enableProxyingToOriginalMethods() - ->disableOriginalConstructor() - ->setConstructorArgs(array( - 'from@example.com', - 'to@example.com', - )) ->getMock(); + $email->setFrom('from@example.com'); + $email->setTo('to@example.com'); $email->setData(array( 'EmailContent' => 'test', )); @@ -188,6 +168,31 @@ class EmailTest extends SapphireTest $this->assertNotEmpty($email->getBody()); } + public function testRenderedSendSubclass() + { + // Include dev theme + SSViewer::set_themes([ + 'silverstripe/framework:/tests/php/Control/Email/EmailTest', + '$default', + ]); + + /** @var Email|PHPUnit_Framework_MockObject_MockObject $email */ + $email = $this->getMockBuilder(EmailSubClass::class) + ->enableProxyingToOriginalMethods() + ->getMock(); + $email->setFrom('from@example.com'); + $email->setTo('to@example.com'); + $email->setData(array( + 'EmailContent' => 'test', + )); + $this->assertFalse($email->hasPlainPart()); + $this->assertEmpty($email->getBody()); + $email->send(); + $this->assertTrue($email->hasPlainPart()); + $this->assertNotEmpty($email->getBody()); + $this->assertContains('

Email Sub-class

', $email->getBody()); + } + public function testConsturctor() { $email = new Email( @@ -460,8 +465,33 @@ class EmailTest extends SapphireTest public function testHTMLTemplate() { + // Include dev theme + SSViewer::set_themes([ + 'silverstripe/framework:/tests/php/Control/Email/EmailTest', + '$default', + ]); + + // Find template on disk + $emailTemplate = ModuleResourceLoader::singleton()->resolveResource( + 'silverstripe/framework:templates/SilverStripe/Control/Email/Email.ss' + ); + $subClassTemplate = ModuleResourceLoader::singleton()->resolveResource( + 'silverstripe/framework:tests/php/Control/Email/EmailTest/templates/' + . str_replace('\\', '/', EmailSubClass::class) + .'.ss' + ); + $this->assertTrue($emailTemplate->exists()); + $this->assertTrue($subClassTemplate->exists()); + + // Check template is auto-found $email = new Email(); - $this->assertEquals(Email::class, $email->getHTMLTemplate()); + $this->assertEquals($emailTemplate->getPath(), $email->getHTMLTemplate()); + $email->setHTMLTemplate('MyTemplate'); + $this->assertEquals('MyTemplate', $email->getHTMLTemplate()); + + // Check subclass template is found + $email2 = new EmailSubClass(); + $this->assertEquals($subClassTemplate->getPath(), $email2->getHTMLTemplate()); $email->setHTMLTemplate('MyTemplate'); $this->assertEquals('MyTemplate', $email->getHTMLTemplate()); } @@ -480,8 +510,8 @@ class EmailTest extends SapphireTest /** @var Swift_NullTransport|PHPUnit_Framework_MockObject_MockObject $transport */ $transport = $this->getMockBuilder(Swift_NullTransport::class)->getMock(); $transport->expects($this->once()) - ->method('send') - ->willThrowException(new Swift_RfcComplianceException('Bad email')); + ->method('send') + ->willThrowException(new Swift_RfcComplianceException('Bad email')); $mailer->setSwiftMailer(new Swift_Mailer($transport)); $email = new Email(); $email->setTo('to@example.com'); @@ -495,7 +525,7 @@ class EmailTest extends SapphireTest $this->assertTrue((new Email)->IsEmail()); } - public function testRender() + public function testRenderAgain() { $email = new Email(); $email->setData(array( @@ -578,4 +608,24 @@ class EmailTest extends SapphireTest $plainPart = reset($children); $this->assertContains('Test', $plainPart->getBody()); } + + /** + * @return PHPUnit_Framework_MockObject_MockObject|Email + */ + protected function makeEmailMock($subject) + { + /** @var Email|PHPUnit_Framework_MockObject_MockObject $email */ + $email = $this->getMockBuilder(Email::class) + ->enableProxyingToOriginalMethods() + ->getMock(); + + $email->setFrom('from@example.com'); + $email->setTo('to@example.com'); + $email->setSubject($subject); + $email->setBody("Body for {$subject}"); + $email->setCC('cc@example.com'); + $email->setBCC('bcc@example.com'); + $email->addAttachment(__DIR__ . '/EmailTest/attachment.txt', null, 'text/plain'); + return $email; + } } diff --git a/tests/php/Control/Email/EmailTest/EmailSubClass.php b/tests/php/Control/Email/EmailTest/EmailSubClass.php new file mode 100644 index 000000000..0a45f5d8c --- /dev/null +++ b/tests/php/Control/Email/EmailTest/EmailSubClass.php @@ -0,0 +1,11 @@ + + + + <% base_tag %> + + + +
+

Email Sub-class

+ $EmailContent +
+ + +