search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 820 forks source link

Email->setHTMLTemplate/setPlainTemplate in combination with SSViewer::get_templates_by_class #11062

Open beerbohmdo opened 7 months ago

beerbohmdo commented 7 months ago

Version 5.x

Description

The method Email->setHTMLTemplate currently only accepts a single relative template name.

Because it must be a string, you cannot pass the response from SSViewer::get_templates_by_class to it.

And because of this if: https://github.com/silverstripe/silverstripe-framework/blob/5/src/Control/Email/Email.php#L416 you cannot even pass the response of SSViewer::chooseTemplate. Because the .ss extension is stripped from the file and will lead to the error "/absolute/path/to/file_without_extension" does not exists

A workaround I found is to add the .ss extension twice ...

$tmpl = SSViewer::chooseTemplate(SSViewer::get_templates_by_class($myClass, '_MySuffix'));
$email->setHTMLTemplate($tmpl . '.ss');
michalkleiner commented 7 months ago

In your snippet I only see the .ss added once and you mention adding it twice. Am I not understanding it correctly or is there something missing?

beerbohmdo commented 7 months ago

The second .ss comes from SSViewer::chooseTemplate. But setHTMLTemplate/setPlainTemplate removes this.

$tmpl = SSViewer::chooseTemplate(SSViewer::get_templates_by_class($myClass, '_MySuffix')); => /absolute/path/to/themes/foo/templates/MyClass_MySuffix.ss

$email->setHTMLTemplate($tmpl); // Error: Unknown template /absolute/path/to/themes/foo/templates/MyClass_MySuffix

$email->setHTMLTemplate($tmpl . '.ss'); // Works because with "/absolute/path/to/themes/foo/templates/MyClass_MySuffix.ss.ss" only the second .ss will be removed.

kinglozzer commented 7 months ago

Semi off-topic, but it might be helpful as an alternative - we’ve all but stopped using Email to render content directly, and instead pre-render and then pass the HTML through like this:

$html = ArrayData::create(['Asset' => $asset, 'Message' => $message])
    ->renderWith(['App\Email\AssetMessageEmail', 'App\Email\Email']);

$email = Email::create()->setBody($html);

It allows us to have a template structure similar to page templates, which can then use $Layout, so we don’t have to duplicate all the boilerplate <table> stuff that emails often need:

templates/App/Email/Email.ss

<html>
<head>
    ... some meta tags, global styles etc might live here
</head>
<body>
    ... shared email header

    {$Layout}

    ... shared email footer
</body>
</html>

templates/App/Email/Layout/AssetMessageEmail.ss

<p><strong>Message:</strong> {$Message}</p>

I wonder if it would be possible to add a setHTMLTemplates() method that accepts an array and could work like this?

GuySartorelli commented 7 months ago

I wonder if it would be possible to add a setHTMLTemplates() method that accepts an array and could work like this?

That sounds like a good idea. I'd recommend we also deprecate the current setHTMLTemplate() method at the same time, and allow people to pass through just a single template to setHTMLTemplates() if they do want to use a single specific template.