- Issue created by @znerol
- @znerol opened merge request.
- 🇨🇭Switzerland znerol
Pushed a draft MR. With this mechanism it would be easy to just setup a default set of environment parameters, then let third parties mess with them before setting up and running code inside the patched environment.
$defaultTheme = $this->configFactory->get('system.theme')->get('default'); $environmentParams = [ new EnvironmentParameter(RenderContextSubstitution::class), new EnvironmentParameter(CurrentUserSubstitution::class, 0), new EnvironmentParameter(ConfigOverrideLanguageSubstitution::class, $langcode), new EnvironmentParameter(ActiveThemeSubstitution::class, $defaultTheme), ]; // TODO: Allow third parties to alter the environment params. // E.g. invoke hook or fire an event $email = $this->environmentManager->executeInEnvironments( $environmentParams, fn () => $emailConfig->getPlugin()->render($emailConfig, $data, $langcode), ); $this->mailer->send($email->to($recipient));
- 🇬🇧United Kingdom adamps
This is a subset of ✨ Create a new mailer service to wrap symfony mailer Active .
Interesting. You have made it very generic and extensible. The downside is complexity: 3-4 times the lines of code compared with the straightforward approach in Drupal\symfony_mailer\Mailer and 9 extra files (plus it needs tests). The call stack becomes a harder to follow with many nested anonymous functions (whereas in fact only the render switch requires that - the others all operate inline).
My feeling is that we don't want such complexity unless we can conceive of a clear use case for it immediately inside Core. I do feel attracted to the idea of having an environment manager separate from the Mailer, and I would simplify it to a fixed set of things that it can alter, i.e. the 4 we already know. If anyone in Contrib really wants to add a 5th param then they can extend the class, and we could write our class to make that easy. Then we only need 2 files, probably less than half the code, and the interface simplifies to something like this (the manager will swap a NULL for the appropriate default)
$this->environmentManager->executeInEnvironment($function, $langcode = NULL, $theme = NULL, $user = NULL)
OR perhaps we could keep
$environmentParams
if you prefer, with string keys['langcode' => 'XXX']
and if they are missing it uses the default??When switching language, I believe we also need to do
$string_translation->setDefaultLangcode($language->getId()); $string_translation->reset();
TODO: Allow third parties to alter the environment params. E.g. invoke hook or fire an event
FWIW I feel we don't want a separate event. We can use the same event as for altering any other parameter on the email. An extra event would be nuisance for code such as DSM+ Mailer Policy that supports altering any of the parameters generically.
- 🇨🇭Switzerland znerol
I moved the language switching to the
language
module. On monolingual sites this is now a no-op.Also found a use case for this in core: HelpTopicSection::renderTopicForSearch().
- 🇬🇧United Kingdom adamps
OK great. I'm not the right person to review the complexity of the implementation, so let's try and get another reviewer for that.
This does seem to meet the needs of the mailer project. I have put in an example in the IS that shows it fits in the context of ✨ Create a new mailer service to wrap symfony mailer Active . Of course this is based on just one possible idea of solving that issue and the details would change, but the principle seems good anyway. Equally it would fit in
MailManager
(not that I propose changing that now!) or DSM+.