Problem/Motivation
I'm building a site which will incorproate a shop built with Commerce which will operate on it's own theme. I need regular site emails to be sent using the default (non-shop) or currently active theme, but Commerce related emails should always be sent using the shop theme irrespective of the active theme (which might be the admin theme if the email is triggered from a staff action such as sending a shipping notifcation or re-sending an order receipt)
Proposed resolution
Add a 'Theme' column to the Module specific configuration, and change MailsystemManager::getMailTheme to accept optional module and key arguments to use this config in determining the theme, falling back to the current behaviour when module is not provided.
Keeping the arguments optional avoids breaking compatibility with other modules that currently use getMailTheme, although they would likely also need to be patched to ensure they behave consistenly (e.g. when SwiftMailer's massageMessageBody method calls getMailTheme to determine relevant theme for attaching CSS)
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | drupal-mailsystem-per-module-key-theme-selection-3166149-3.patch | 5.72 KB | pstewart |
Comments
Comment #2
pstewart commentedI've had a go at implementing this, will post a patch shortly
Comment #3
pstewart commentedHere's the patch. I've tried it out, seems to be working as expected.
Comment #4
berdirHm, that does add quite a bit of complexity and UI elements. I also just realized that the form might have a bug that doesn't allow you to unselect the value of an element as we only save it when we have a value? (not just for the new theme key).
Wondering if we should just make this an API that's easy to extend, in a separate service for example, if someone needs this flexibility, they could replace/decorate the service?
Comment #5
pstewart commentedI've confirmed that unselecting does work correctly - the existing logic in
submitFormalready takes care of this. I've just tested this out by selecting '- Default -' and saving, then doing a config export to confirm the corresponding field is indeed being removed from the row.The logic added to
MailsystemManager::getMailThemeis copied from the corresponding logic ingetPluginInstance, so we're not really adding much more in the way of complexity in that sense. I agree that the admin UI gets a little busier with the extra column, but I don't think it's overly complex - it's a reasonably intuitive expansion on the current interface (IMO perhaps more intuitive - I found it a bit strange that I was presented with the ability to configure defaults for three things at the top of the form and then only override the defaults at the module / key level for two out of three of those things).I think the proposed solution is a reasonable expansion of the current functionality and increases the range of scenarios the interface can provide a solution to without site builders having to reach into code. It does feel like this would be the limit for the config interface though, certainly an API would be the way to go for any more complex scenario.
Comment #6
berdir