Wrong email builder chosen

This can't be fixed until 2.x. Created 📌 Warn about unsupported override combinations Fixed to warn about it in the meantime.

We 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 the LegacyEmailBuilder class).

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:

1. The discovery specified in DefaultPluginManager discover all the available email builder plugins.
2. EmailBuilderManager::processDefinition() processes the definitions. In case of the ContactPageEmailBuilder plugin, the plugin ID contact_form also must contain the config entity type ID associated with the email builder plugin (the has_entity annotation is set to TRUE), ::processDefinition checks whether there is a entity type with ID contact_form available or not. It isn't (we don't have contact module being enabled), so the provider in the definition is set to _.
3. symfony_mailer_mailer_builder_info_alter start to alter these definitions.
4. Our module's hook_mail() implementation gets discovered, but since Drupal Symfony Mailer already defines a plugin with ID contact_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!
5. Since the contact module (what contact_form builder plugin provider is '_', it gets removed in DefaultPluginManager::findDefinitions().
6. So when we try to send an email through that module with MailManagerReplacement::mail(), EmailBuilderManagerInterface->createInstanceFromMessage() wont't find any matching email builder plugin, so it doesn't return anything (which is actually equal to returning a NULL in 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 === module name, utilize the provider annotation 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 using provider is enough.

• Instead of assuming that the related entity type is the same as the plugin ID (as I see this happens if has_entity is set to TRUE), add a 'content_entity_type_id' (or something similar) annotation property (and maybe deprecate 'has_entity'). BTW, does this module uses this info for anything else than calculating the provider? (Didn't checked yet, just thinking loudly.)
• Change logic behind EmailBuilderManager::createInstanceFromMessage()::
  • Instead of assuming that plugin IDs are matching $module or $module.$key pattern, find the corresponding plugin based on the provider annotation 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 like mymodule.my_email_key, but it won't be a must.) AND
  • Make EmailBuilderManager using FallbackPluginManagerInterface, and convert LegacyMailBuilder into a real (annotated) plugin - which then can be used as the fallback builder plugin (e.g. this is what the Block(Plugin)Manager does in Drupal core). This can eliminate the requirement on the logic implemented in symfony_mailer_mailer_builder_info_alter.

@AdamPS, what do you think?

@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 🐛 Disabled overrides should not cause conflicts Fixed - please continue the discussion there.

LegacyEmailBuilder doesn't have @EmailBuilder annotation because we want to create multiple instances of it with different id, label and provider - see symfony_mailer_mailer_builder_info_alter(). These instances are used by the mailer policy form and perhaps other places.

@huzooka Please can you test the patch on 🐛 Disabled overrides should not cause conflicts Fixed ?

Fixed by 📌 Improvements to email interface Active , but excluding the update hook.