Problem/Motivation
If override is enabled for "Simplenews" but not for "Simplenews newsletter" then emails for "Simplenews newsletter" will be routed instead to "Simplenews". The email fails to send with this error message:
Error: Call to a member function getMail() on null in Drupal\symfony_mailer\Plugin\EmailBuilder\SimplenewsEmailBuilderBase->build() (line 25 of modules/contrib/symfony_mailer/src/Plugin/EmailBuilder/SimplenewsEmailBuilderBase.php).
A similar problem occurs if override is enabled for "Personal contact form" but not for "Contact form".
Steps to reproduce
See above
Proposed resolution
The email builder id is currently simplenews which picks up all simplenews emails. The only solution I can see is to change the email builder IDs:
- simplenews => simplenews_subscriber
- contact => contact_user
Unfortunately this is non-BC. The update hook renames existing mailer policies. However sites needs changes to templates and email alter hooks.
Remaining tasks
User interface changes
API changes
Change to the email builder IDs in email alter hooks.
Data model changes
Change to the email builder IDs in mailer policy.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | symfony_mailer.wrong-builder.3366091-3.patch | 9.55 KB | adamps |
| #3 | symfony_mailer.wrong-builder.3366091-interdiff-2-3.txt | 3.81 KB | adamps |
| #2 | symfony_mailer.wrong-builder.3366091-2.patch | 8.86 KB | adamps |
Comments
Comment #2
adamps commentedComment #3
adamps commentedComment #4
adamps commentedComment #5
adamps commentedThis can't be fixed until 2.x. Created #3367314: Warn about unsupported override combinations to warn about it in the meantime.
Comment #6
huzookaWe just run into this issue with 1.3.0 (issue still seems to be present in 1.x too).
We have custom module named
contact_form. If we try to send an email on behalf of this module, Drupal Symfony Mailer won't find a matching email builder (nor a dynamically created instance using theLegacyEmailBuilderclass).How we worked around this temporarily
We prefixed the mail module param with an extra string, and we added an email builder plugin with the mathcing ID.
Analysis
This happens because we won't have any builder plugin definition for this module. Why? Let's see what happens during the discovery phase under the hood:
DefaultPluginManagerdiscover all the available email builder plugins.EmailBuilderManager::processDefinition()processes the definitions. In case of theContactPageEmailBuilderplugin, the plugin IDcontact_formalso must contain the config entity type ID associated with the email builder plugin (thehas_entityannotation is set to TRUE),::processDefinitionchecks whether there is a entity type with IDcontact_formavailable or not. It isn't (we don't have contact module being enabled), so the provider in the definition is set to_.symfony_mailer_mailer_builder_info_alterstart to alter these definitions.hook_mail()implementation gets discovered, but since Drupal Symfony Mailer already defines a plugin with IDcontact_form, the logic inside the foreach loop won't generate builder plugin instance for our module - because there is already a builder plugin with that ID!contact_formbuilder plugin provider is '_', it gets removed inDefaultPluginManager::findDefinitions().MailManagerReplacement::mail(),EmailBuilderManagerInterface->createInstanceFromMessage()wont't find any matching email builder plugin, so it doesn't return anything (which is actually equal to returning aNULLin this case) ➡️ we get the same error message as in the IS (but for a bit different reason).Suggestion for a BC-safe fix
Separate module name / builder plugin ID / ID of the related entity type:
Instead of assuming that
plugin ID === modulename, utilize theproviderannotation property: the provider could be added to the plugin annotations:provider => "contact". Any module can define plugin on behalf of other modules! This could replace the problematic parts in point 2.If this turns to be trickier than ideal, introduce a '
provider_module' or 'module' annotation instead, but I think that usingprovideris enough.Instead of assuming that the related entity type is the same as the plugin ID (as I see this happens if
has_entityis set to TRUE), add a 'content_entity_type_id' (or something similar) annotation property (and maybe deprecate 'has_entity'). BTW, does this module use this info for anything else than calculating the provider? (Didn't checked yet, just thinking loudly.)EmailBuilderManager::createInstanceFromMessage()::$moduleor$module.$keypattern, find the corresponding plugin based on theproviderannotation of the builder plugin (and also checking the sub_types if any). But! The current approach could be kept as fallback (so we can still use plugin IDs likemymodule.my_email_key, but it won't be a must.) ANDEmailBuilderManagerusingFallbackPluginManagerInterface, and convertLegacyMailBuilderinto a real (annotated) plugin - which then can be used as the fallback builder plugin (e.g. this is what theBlock(Plugin)Managerdoes in Drupal core). This can eliminate the requirement on the logic implemented insymfony_mailer_mailer_builder_info_alter.@AdamPS, what do you think?
Comment #7
adamps commented@huzooka interesting, thanks for the report. I agree that the problem you see has the same symptoms as this issue. However it's a separate bug as it has different steps to reproduce, and will have a different fix. Therefore I raised a new issue #3379529: Disabled overrides should not cause conflicts - please continue the discussion there.
LegacyEmailBuilderdoesn't have @EmailBuilder annotation because we want to create multiple instances of it with different id, label and provider - seesymfony_mailer_mailer_builder_info_alter(). These instances are used by the mailer policy form and perhaps other places.Comment #8
adamps commented@huzooka Please can you test the patch on #3379529: Disabled overrides should not cause conflicts?
Comment #9
adamps commentedFixed by #3480860: Improvements to email interface
Comment #10
adamps commented