🇬🇧United Kingdom @adamps

I created #3557360: Support retraction → for retraction. I planning to start on this one next.

adamps → created an issue.

We are using the status in several places. However AFAICS we don't anywhere rely on it being a field. Here's a MR that tests removing the field but keeping the functions.

It's now simpler than it was when I raised the issue. I see a difficultly that this change is complex and unmaintainable - the isOngoing() function can change, and we might forget to change the query, or make a mistake so the query is incorrect in edge cases.

We can come back to this if we see actual performance problems.

The test no longer uses ajax, so I converted it to a functional test and fixed the code.

Mostly now code complete. I deferred one part ✨ Allow declaration types to specify field defaults Active .

adamps → created an issue.

Send confirmation corresponding field on the declaration: missing

Current status: basics are roughly working, tests pass, but missing most of extras.

1. A mechanism for selecting declaration bundle type: currently using the default add-page route. It doesn't show the description. It requires an extra page transition. The donor context gets lost.
2. A mechanism for setting default declaration field values based on the bundle type: missing.
3. Per-bundle permissions: missing.
4. Used a method field copied from evidence types (combines first 2 of your fields)
5. Send confirmation: actually called written_record_required, copied from the code we had before
6. Provided types: user, commerce_order both hidden; "Recording" so there is at least something there by default, and the tests need it (but I could put it into test code only)

OK thanks. You could do a bulk post-commit review again - it seemed to work well last time.

If you aren't able to review fairly soon then I likely will need to commit so that I can continue on other issues, and then you can review after

adamps → changed the visibility of the branch 3536681-more-robust-validity to hidden.

OK, thanks for explaining. Sure, I can see the advantages you refer to. So the 3 states would be

1. Invalid
2. Valid if confirmed
3. Inherently valid

I adjusted the names slightly, looking for something that conveys the idea that the second one would in the majority of cases be valid, as normally we would sent confirmations promptly.

At the UI level, for the status field, then I feel that we still need to stick with the original meanings - the last 5 states are all sub-cases of valid, so we don't really have a way to convey the exact details of how it came to be valid.

At the API level, isValid() would map to a simple calculation: "Inherently valid" OR ("Valid if confirmed" AND "Confirmation sent"). I agree that we could make a computed field for this later, however we don't need to now.

I don't have any magic answers.

The Core developer(s) seemed to prefer to invent a new solution, even though a successful one existed in contrib. For sure I spent hours trying to establish a useful discussion, but my comments were mostly ignored. At the time that I left the team there was no sign of any compatibility at all with this module - it was different at every level.

It's not really a matter of anybody else being able take over maintaining this module. The problem is that the whole basis that this module relies on will likely be gone: the old mail interface (used by the legacy support) will be removed and the new one in core will be entirely different from here. Perhaps this module could still exist for any modules that specific choose to use the interfaces from here rather than the new Core ones, but it potentially won't be attractive for modules to do that.

So, you are reliant on what the Core team produce, and whatever new contrib modules someone else writes. Those are the people who might write the upgrade paths. The new contrib modules can probably copy some parts from here, but it's not really any more the same module, so probably best for people to start new ones. In that way I can ensure this module stays stable for those who want it in the current form, whilst leaving the new team free to do whatever they wish on the new start.

I probably will make a stable 2.x release as the branch has many good bug fixes. However it is significantly non-BC, so any code that uses the interfaces from here would need to be updated. Given the dubious future, people may not wish to. However if you don't rely on any code like that and just use this module directly then 2.x could be a better option.

I added a link from the project page to this issue. If you have concerns then I suggest you raise them in the Core issues.

It works fine for me. The patch makes no sense to me. The code contains no reference to stream_options, auth_mode or others in the IS.

The MR seems to contain no fixes. All fixes need to go into 2.x please.

It's not necessarily a bug - this module defines its own events that are more flexible and powerful than the base ones. Why would we need both??

Done

Thanks for the report.

Normally it is set by Drupal\symfony_mailer\Attribute\Mailer, however the *All* and *Unknown* policies don't have a definition - see MailerPolicy::parse().

The MR uses a case-by-case approach to handle the missing definition. However we can't know for sure that this is the end of the problem - there might be cases we didn't spot with other keys, or new ones might get added in the future. I would prefer instead an approach that creates an "empty" definition initialised with default values. This code be done with code similar to OverrideManager::getLegacyMailerDefinition(), except we don't need a subdef and we can't use LegacyMailer as the class.
Perhaps we could add a function MailerManager::createDummyDefinition() equal to most of getLegacyMailerDefinition() which then becomes a thin wrapper.

I added a paragraph and a ref to ✨ Allow node/entity to display title/label field as normal Needs work to the project page.

Is there a reason for having #access=false by default?

It comes from Core EntityViewController::buildTitle().

Should we add an issue for it?

There already is one ✨ Allow node/entity to display title/label field as normal Needs work

Or perhaps there could be a configuration form somewhere to allow access?

Yes that would be part of that issue

Seems crazy to me that I have to add that or the title field is hidden.

Yes, but the craziness came from Core. This module is not causing your problem - it is part of the solution to your problem.

I can confirm that #5...

#5 is a good workaround. It's not something we can commit for the reasons in https://www.drupal.org/project/manage_display/issues/3143678#comment-158... ✨ Allow node/entity to display title/label field as normal Needs work .

This module aims to be absorbed into Core, please see the project page.

The title field is covered by Core issue ✨ Add a tag select list in string Formatter Needs review , which is nearly done. It just needs an extra test. If anyone wishes to support this module/feature, then that would be a great way to do it. Once that fix lands, we will eventually remove the title formatter here, and add an update hook to convert existing sites.

Core has already chosen the list of tags, see https://git.drupalcode.org/project/drupal/-/merge_requests/9341/diffs. This module needs to stay in sync, and match core. If we allow an extra tag, then it creates a problem writing this update hook.

So sorry we don't wish to add this fix. However a p tag mostly just affects spacing, so you might be able to use the existing string formatter with some CSS.

It sounds like you need composer advice - it's not a question for this issue queue.

It's not an "oversight"😃. There are many fields that Core chose not to make display configurable. This module doesn't attempt to expose them all, which in many cases anyway wouldn't be desirable, as the core maintainers had a reason for their choice. If a site doesn't agree with the decision made by Core, then it can change things using hook_entity_base_field_info_alter().

This module has a specific purpose to fix some old hacky code. Some key fields are missing from "manage display" configuration, and their display is hard-coded into the template. Fixing this is complex, and it requires special attributes such as enable_base_field_custom_preprocess_skipping and enable_page_title_template. This module specifically aims to expose these key fields.

Thanks. Would be great if someone else could test.

#13 is still open. It could be put into a separate issue, but would be nicer to add it here if possible.

This is the last project blocking my D11 upgrade😃. I hope we might get a new release soon please as many sites don't want to put a dev release onto a live site. I understand, folks are busy, but maybe it only takes 5 minutes.

It's a good theme, but eventually folks might drift away...

Thanks to @znerol #51 I realised that DSM+ doesn't need to use plugins for Mailer/MailHandler services. I still propose to use attributes on the service class for the metadata, which can be discovered by means of a service tag. I raised 📌 Mailer classes don't need to be plugins Active . I agree that Yaml files could also work to define metadata.

Commerce created an improved mail system and AFAICS it has the following benefits: it allows a mail to be defined in a single file; it implements language switching; it uses twig templates to define the email (as does simplenews). Easy Email provides a UI for configuring email body. DSM+ supports all of the mentioned features and emails use twig either from config or a template.

We can follow these successful examples. We can combine the MailHandler and MailBuilder into a single class. The building part by default lives in a function build(EmailInterface $email), or the class can define multiple build functions. It is similar to a Form, which can have multiple submit functions. We no longer need the _controller in the Yaml file because the MailHandler is self-contained and specifies its own build function. Code that wishes to replace the implementation can override the entire MailHandler service. This avoids many of the problems of #53 which come from replacing half of the implementation. Code that wishes to alter the email registers their own build() function which receives the identical information to the original build. The build() function would normally not set an HTML body render array (although it could using setBody()) because mostly it seems better to define the HTML using twig config/template rather than as a series of calls to t() in code.

I admit, my proposal resembles DSM+😃. However it is proven to work.

adamps → created an issue.

We have requirements [D2 CAN_REPLACE] and [D3 CAN_ALTER]. I would be interested to understand how they are met by 📌 Introduce email yaml plugin and an email building pattern known from controllers Active .

1. I would like to override ContactMailBuilder and trust that its public methods won't change (except in ways that are BC).
2. I would like to be able to access or change any aspect of the email, including Drupal specific concepts not present on the Symfony class. I might have previously altered or added to the email parameters or wish to do so in this code. So I would like the build functions to have just one parameter, which is the Drupal EmailInterface and then I can get everything from that, including parameters, and key. (The following comments give more detail about specific parameters.)
3. I would like to use the current langcode (I had the opportunity to participate in language negotiation, which was followed by environment switching) rather than having a $langcode parameter.
4. I would like to be free to build the email however I choose, which doesn't necessarily use a filter format, so don't wish to receive a $format parameter. I understand that I could ignore it, however I feel that the presence of the parameter on the interface gives the calling code a false expectation that it will have an effect.
5. I would like to generate my own body and subject however I choose, including generating it from a Twig string in config or using a Twig template. I don't wish to replicate the code that sets up the variables instead I would like to access them via the Drupal EmailInterface, and trust that the set of available variables won't change (except in ways that are BC). I would also like metadata that describes the available variables which I can show in the UI.
6. I would like to override User mails too and I would like a consistent pattern compared with other modules, in other words to find a mail builder. I will assume for now that the UserNotificationHandler->build() could be moved to a mail builder.
7. I would like to build the mail how I choose. I don't wish to receive parameters $confirm_url and $token as they seem to correspond to an implementation detail. I don't wish to give the calling code the idea that it can choose these values (AFAICS doing that would break even with the Core implementation). I would like to choose which emails contain the confirmation URL.
8. I would like to customise the replyTo address, including the possibility to have none. Therefore I would like the code that sets replyTo to be in the mail builer which I can then override rather than in the calling code. Similarly for the address/langcode of the admin email, the recipients of an update email and the decision whether a particular user mail is enabled. I would like to create a unified UI that can configure any value on any email, including the decision to enable/disable it.

#51 is very useful thanks. I expect that if someone wrote a similar response to #51, and "Drupal adaptation layer" it would also be useful.

@johnny5th Good point. We can check if the route exists, or just catch the RouteNotFoundException.

The routing.yml file is flexible and it can map to many "things" - not only to a controller but also a form, an entity list, and entity form, an entity view, and maybe more. In the case of emails, I feel that we only have "thing" and so we don't need the flexibility. There is another pattern which is more common/familiar and simpler: plug-in plus attribute. We could make an @EmailBuilder plug-in and put the metadata (currently in the yaml file) on the attribute.

The current controller design only covers the email body. To build an email we also need to cover the subject, recipient and perhaps more. These must also be set within the switched environment because they are language dependent, and can affect render context. I feel we should let go of the idea of returning a render array (even though it does give a pleasing similarity with the routing file) and instead the controller should act on the Drupal EmailInterface object, typically in the build phase, but could also be init or post-render. This gives the entire implementation in a single class which is easily swappable.

Great, we now have several sources of ideas: 📌 Introduce email yaml plugin and an email building pattern known from controllers Active ; DSM+; Easy Mailer (although AFAICS it mostly acts at higher layers than what we are concerned with here); comments on this issue and on others.

I feel that the key to moving forward is to take some decisions as a group, specifically related to the requirements and design. I added a new section to the IS "Key decisions and resulting issues" with 3 draft proposals. They are related, and could alternatively be seen as one proposal, to create layer 3, the "Drupal adaptation layer".

In my view this is the most important part of the new mail system. It's the absolute minimum that we add on top of Symfony to get a useful working Drupal integration. It exposes an API still quite close to the original Symfony without a load of "baggage".

I fixed the tests - I added some asserts that fail before the fix is applied. The fix is now ready for commit.

Unfortunately the tests are failing on the 4.x branch, see 📌 Fix tests Active , so we need to wait for that to be fixed.

There is one easy fix: add static to public function simplenewsNewsletterNameTokenDataProvider() in 2 places.

The other failures relate to the simplenews_issue field being missing, which is strange. It should come from /config/optional/field.field.node.simplenews_issue.simplenews_issue.yml.

adamps → created an issue.

Regarding $langcode, my comment is purely about where we put the code, and I have no intention to change the behaviour.

On MailManagerInterface, the call site code is forced to set $to and $langcode, and optionally can set $reply. All other email data is set in hook_mail(). This causes two problems:

1. Hook code cannot easily alter the language - it would need to duplicate the logic in user_mail()
2. There is a split in the code for building the email, and anyone reading the code has to look in two places.

In DSM+ the email builder plug-in builds the entire email. It uses the init() function to set any values that affect context switching and the negotiated language is still available. It uses the build() function for everything else.

I believe my proposal helps to fix issues like 🐛 Password reset invalid mail notify language Needs work . We can centralise the automatic choice of language for sending to a user inside the mailer service, based on extending setTo() to accept a user object. We can even generalise this further by introducing a EmailableInterface which classes such as User and Subscriber can implement (it has functions to get the email, display name, language and user context). This avoids requiring every module that sends email to a user to duplicate the same code (or to use different code and give inconsistent results). So we could decide that the correct code is something like this: IF "the detection method is based on URL" AND ("current user matches the target user" OR "current user is anonymous") then we pick the current language, else we would pick the user language.

I feel that in most cases the email building should not be context dependent. For example, if the admin resends the commerce order receipt, then it should be the same as the original. If a test newsletter is sent then it should be identical to the actual email that will eventually be sent in cron. If a module triggers generating of a user mail (currently by calling _user_mail_notify()) then most likely they wish to use the same language as would normally be used.

Please see #7, especially

There is no point in having a debate as we can't change it. Any change would alter the emails of all the existing sites in ways that could be undesirable.

And should relatively mean any existing HTML emails remain in-tact.

Not true. Your suggestion would add paragraph or break tags wherever this is a line break. Whitespace is not supposed to have meaning in HTML, and this change would break the layout of any email that happens to contain a line break.

The processed_text rendering that occurs is already doing _filter_autop but it also strips all HTML and recreates links (it falls back to the fallback format that is usually plain text). So for MarkupInterface would be bad as it will strip HTML (though arguably does any Legacy email support it?).

Yes but the processed text rendering only occurs for plain text emails (not instance of MarkupInterface), which is the correct behaviour.

I propose to add #3542264

Makes sense to me

The contrib module is currently not compatible with D11 and also has only 34 sites that report using it.

When the mailer ceases to be experimental, do we still need a Core module? If so, then I feel we should keep the name "mailer". If this is only a temporary module, then one option is to give it a name that indicates the temporary status.

Thanks. It's a fairly unusual case because symfony_mailer_update_20001() enabled the mailer_policy module on all sites. However it's a valid bug. Presumably the same problem exists with symfony_mailer_update_20004().

The solution should be easy - just add

if (\Drupal::service('module_handler')->moduleExists('mailer_policy'))

The MR appears to have no commits.

Is this still an issue?

Thanks for checking. I no longer use this module, so please feel free to close the issue.

Our code is experimental, so we are allowed to change things after commit. Even so I feel we should do our best to get things right first time to minimise confusion/disruption for those evaluating/testing the experimental module. Is there are reason why we can't do that here? We are implementing the test API which will become widely used in module test code.

1) The mailer capture transport should always be enabled when running tests using the experimental mailer (same as in the old mail system). We don't need to ask developers to enable a test module to make it work.

I hear the discussion about possible technical difficulties, however surely there must be a way as we can change any code that we like. There are some suggestions in #27

We could work around that with some hardcoded logic that sets capture back to null and say we don't support that there. Or we actually do add support for that in there ourself. It is just another transport.

2) Change the trait name now so we don't have to rename it in ✨ Framework for testing Symfony Emails Active . Our goal is one trait that covers both capturing and asserting - tests will always need both and they don't need to use two separate traits. (Perhaps a unit test only wants asserting, if we ever write one - fine it can just ignore the capturing part.) I propose MailerTestTrait, which matches DSM+ and is similar to the name AssertMailTrait in the old mailsystem. If you prefer a different name that's OK by me if it covers both parts: capture and assert.

Then in the IS "proposed resolution" we can remove the reference to the mailer_capture module and instead add a reference to the trait.

How do you feel about that??

The yaml plugin is a really interesting idea that I would like to discuss and I believe it could work well. I find it difficult to discuss in the context of 📌 Introduce email yaml plugin and an email building pattern known from controllers Active because that contains a lot of details also about 10 or 20 other ideas, some of which I find good, some bad. As I said before, I propose we will make progress by taking decisions one-by-one.

I feel that the Mailer service from ✨ Create a new mailer service to wrap symfony mailer Active is a good match with the yaml plugin. The Mailer interface could be written as

function send($id, $params)

And the call-site code then becomes attractively simple

class UserNotificationHandler {
  public function sendRegisterAdminCreated(UserInterface $user): bool {
    return $this->mailer->send('user.register_admin_created', ['user' => $user]);
  }
}

All the work is now in the "controller" part, the mail builder, or whatever we call it. It's easier to understand in one place, and it's easier to swap the implementation (compared with the old mail system, where part of the code is in _user_mail_notify(), part is in user_mail() and neither can be swapped!).

We could even add parameters to the yaml file

    parameters:
      user:
        type: entity:user

although perhaps even better we could automatically deduce the type because the name is an entity type provided by the same module.

AFAICS 📌 Introduce email yaml plugin and an email building pattern known from controllers Active covers layers 3 to 5.

If we implement layer 3 (Drupal Adaptation) it would likely take in EmailRenderer. It would simplify the rest of the code quite a bit. We could remove all reference to $langcode because the language would already be switched. We could integrate concepts like token replacement into the underlying service.

I had a first go to understand, and found it not so easy😃. I feel that for a module to send an email should not be so complicated. It seems much simpler in DSM+. Could you help me compare them? Either we could make this simpler, or list clear reasons why it is more complex.

1) These lines seem to happen a lot. I feel it should be just one line to send a mail. The underlying layer can handle the separate build/render/send.

try {
        $email = $this->emailRenderer->render($definition, ['items' => $items], $langcode);
        $this->mailer->send($email->to($recipient));
        $sent++;
      }
      catch (TransportExceptionInterface | PluginException $e) {
        Error::logException($this->logger, $e);
    }

2) I'm confused by inconsistencies between the modules, they seem far from following a pattern. Some have a builder some not. I feel that one class is enough and it can have separate functions for the parts. All modules can have the same 2 functions.

@zengenuity and I had a useful discussion of the design. We tended to agree at the high level, then had different terminology and ideas at the detailed level. I adjusted the IS proposed resolution slightly, including to reverse the order of presentation (sorry!) because this will match the order we work in - continuing our journey up the layers, each one dependent on the ones below. The next layer is ...

3) The "Drupal adaptation layer". We already have detailed issues with a list of points for discussion in the proposed resolution. Following @berdir suggestion I feel we should combine them into one (which it was originally!). ✨ Create new mailer service based on symfony Active , ✨ Create a new mailer service to wrap symfony mailer Active , ✨ Events/callbacks for new mailer service Active .

4) Then I propose that the final layer for Beta would be the building layer.

We can leave the others....

5) In the previous meeting and in a previous prototype I recall that we had the idea that for the Beta we could stick with the existing config layer. So Core generates plain text mails, and Contrib can override for HTML. I have kept that idea as a proposal in the IS.

6) @znerol already prototyped how we can avoid changing the user layer for beta, by overriding the old mail system services. I added that to the IS, it seems clearly good.

Before we move on to work on the individual layers, please can people comment on how they feel about the overall picture of the layers, and what we would do in our first beta version??

It's not really part of the work on the new mail system at all, it fits more into the work to deprecate and move all functions in .module files to services/classes.

Thanks that is a helpful way to describe it.

This is ready from my side.

OK yes it makes sense. I am convinced, and I believe the others on the thread were already convinced a while ago.

I added some new requirements based on recent comments and some of my own ideas, all marked DRAFT. I propose that one existing requirement be deleted, marked DELETE.

Please comment to say whether you agree, or alternatively we could do it at our next meeting.

I think we cannot deprecate _user_mail_notify as a whole without providing proper APIs to cover most of these use cases first. That said, I think the best way forward would be to deprecate custom ops here in a first step.

I feel that we have an easy solution. In this issue we could create NotificationHandler with exactly the same interface as _user_mail_notify(). The only difference is that it is a service with an API rather than an internal function. The purpose of the change is to prepare for mail system migration. It becomes just the same as contact module and others.

Then later in the new mail system we switch to the new preferred interface of one function per op, we change the return code, and anything else that we want. This follows the basic principle that we avoid change to the old mail system except in rare cases that we really have to.

Great thanks @znerol.

In the new mail system, Contrib would like to be able to trigger generation of emails.

I realise that the same applies to the old mail system. That's why we have lots of calls to _user_mail_notify() even though it's technically internal.

We are deprecating _user_mail_notify(). What do we expect people to use instead? The only answer I can see is NotificationHandler. Even if we mark it internal then developers will call it as they have no choice. So I suggest that it should not be @internal and should have an interface. This would match the other existing mail handlers.

When the new mail system becomes stable then we would deprecate NotificationHandler. The developers will have to change their code again, but I don't see an easy alternative.

EDIT NB My comment was a cross-post with the previous one.

Thanks for the clarified IS. I now understand much better what we are doing here, sorry for the prior confusion. I have made two clarifying statements below. Provided that you agree with these, then +1 from me.

The new service is marked @internal and intentionally has no interface. It is discouraged to use the service outside of core. The only purpose is to simplify transition to a new mail system.

In the new mail system, Contrib would like to be able to trigger generation of emails. I believe Easy Email might include a UI for this feature generally. Commerce has it specifically as "resend receipt".

_user_mail_notify has responsibilities which shouldn't be substituted. E.g., it has code which checks whether a notification is enabled and it logs an error if an account has no email configured. A contrib/custom module would need to replicate and maintain these responsibilities over time. This isn't desirable.

In the new mail system, Contrib would like to be able to substitute any part of the email code, and we have requirements relating to this. I wrote in more detail about this as a comment on the MR.

Without experimentation its impossible to know what works and what breaks, though.

We have one 4 year experiment including over 40k sites that is known to work😃.

In 📌 Introduce email yaml plugin and an email building pattern known from controllers Active :

The rendering itself happens inside an EmailRenderer service which has the following responsibilities:

That sounds rather like ✨ Create a new mailer service to wrap symfony mailer Active :

The Email building pipeline proceeds in the following sequence:

Also I see you are introducing template suggestions, email plugin manager, some metadata on the plugin definition, and various other things quite similar to DSM+. You have some really interesting new ideas that I like. So that is quite promising. On the other hand, I can see many ways in which DSM+ is fairly obviously better, and the MR code would fail to meet needs that have been clearly expressed by multiple developers in Contrib issues.

Many of my comments remain unchanged from before. I find these huge MR too big to process in any sensible way. There seems to be a danger in "reinventing the wheel". We should also consider simplifying the transition process for the existing sites, so try to avoid replacing an interface with something mostly equivalent, when perhaps we could leave it the same and it be equally good.

I feel that likely you experiment with these big prototypes to clarify your own thinking, and to explore radical new options. Which is fine, but eventually we need to find our way to agreement on issues we could commit.

Here are some steps that I would see as useful:

• Start discussion on the "Drupal adaption layer" to create a Drupal Mailer service and EmailInterface. These map very clearly to many of the agreed requirements, and I feel are essential. It would significantly simplify all the MR you create, because currently you have a lot of "boiler plate" code that could be handled by this layer.
• Work on details in specific issues with a smaller scope. This would help me even if @berdir is right and we have to bring it back together to get a commit. Obviously this requires some agreement on a design, which I am keen to discuss.
• Provide a clear explanation for the choices made, especially when it is re-inventing very similar proven Contrib code. If you could reference a DSM+ class/concept/etc and specify 3 improvements that you would like to make then I can likely easily agree. I would then point out 5 things you apparently made worse, you might agree in 4 cases, it was just an accident, we discuss the 5th case, and very quickly we are done.
• Reuse or reference existing issues and ideas from other people rather than duplication.

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+.

My interpretation of this part about the correct line-endings is about ensuring that the email is valid. It is saying that the calling code doesn't have to worry about the details in the RFCs regarding the rules for line-endings. For an HTML email, then the body is MIME-encoded. Still, Drupal (or in fact Symfony Mailer Library) will put line endings according to the rules, but they won't have any effect on the message.

I agree that this isn't the only way to interpret it. Anyway the comment in hook_mail was likely written before HTML email was even invented. There is no point in having a debate as we can't change it. Any change would alter the emails of all the existing sites in ways that could be undesirable. I feel happy with the current version which matches behaviour elsewhere in Drupal e.g. hook_help().

The only solution I can see is for you to add in a space or line break in your code.

PS If you Mail System, then you still need to use something else as a plugin to send the emails. Mail System just allows you to configure which thing to use.

We have an amazing opportunity to rewrite the entire Drupal mail ecosystem without constraints from the past. I can see so many possibilities😃. We could automatically generate the entire Email Configuration UI from simple metadata on the mailer service/plugin classes.
We could make the email building code extremely simple and clear by including a few common patterns into the Core framework (and avoid bugs where people forget those things).

We can make the migration process simple for both the site owner and developer with a carefully designed migration framework. Given that we don't have much idea yet what the migration process will be, how do we know that this issue is going to be helpful (please see #4)?

DSM+ is an example of many such things. Sure maybe we won't to do any of it in Core, but it would be great if we could at least take time to have a look at them and decide. This issue ends up constraining things that we might later want to change. We are in danger of eliminating possibilities without most people even knowing what those possibilities are.

Below is the user email code from DSM+. It replaces _user_mail_notify(), _user_mail() and most of AccountSettingsForm. See all the code that's just gone away:

• language is automatically switched based on the 'to' address (which accepts AccountInterface)
• token replacement is automatic from the token_types metadata (we only have to set the special token options)
• subject/body config is stored so it can be automatically applied (the config entity id specifies the applicable mail tag, the fields specify the different parts of the email)
• same for the 'to' address config for the admin message, defaulting to the site address
• the config UI can be automatically built (including appropriate token help) based on the required_config metadata, we just need a single line in AccountSettingsForm to trigger its placement

All we have left is this below, the simple essentials. Of course it doesn't need to be exactly like that - there are many possible variations and improvements that follow the same ideas.

declare(strict_types=1);

namespace Drupal\symfony_mailer\Plugin\Mailer;

use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\symfony_mailer\Attribute\Mailer;
use Drupal\symfony_mailer\EmailInterface;
use Drupal\symfony_mailer\Processor\MailerPluginBase;
use Drupal\user\UserInterface;

/**
 * Defines the Mailer plug-in for user module.
 *
 * Replaces _user_mail_notify().
 */
#[Mailer(
  id: "user",
  sub_defs: [
    "cancel_confirm" => new TranslatableMarkup("Account cancellation confirmation"),
    "password_reset" => new TranslatableMarkup("Password recovery"),
    "register_admin_created" => new TranslatableMarkup("Account created by administrator"),
    "register_no_approval_required" => new TranslatableMarkup("Registration confirmation (No approval required)"),
    "register_pending_approval" => new TranslatableMarkup("Registration confirmation (Pending approval)"),
    "register_pending_approval_admin" => new TranslatableMarkup("Admin (user awaiting approval)"),
    "status_activated" => new TranslatableMarkup("Account activation"),
    "status_blocked" => new TranslatableMarkup("Account blocked"),
    "status_canceled" => new TranslatableMarkup("Account cancelled"),
  ],
  required_config: ["email_subject", "email_body"],
  token_types: ["user"],
)]
class UserMailer extends MailerPluginBase implements UserMailerInterface {

  /**
   * {@inheritdoc}
   */
  public function notify(string $op, UserInterface $user): bool {
    if ($op == 'register_pending_approval') {
      $this->newEmail("{$op}_admin")->setEntityParam($user)->send();
    }

    return $this->newEmail($op)
      ->setEntityParam($user)
      ->setTo($user)
      ->send();
  }

  /**
   * {@inheritdoc}
   */
  public function init(EmailInterface $email): void {
    $email->setParam('token_options', ['callback' => 'user_mail_tokens', 'clear' => TRUE]);
  }

}

Thanks.

I found that using splide-wrapper class without a nav already breaks the base splide library. I didn't fully understand it, but I guess it might be from splide.load.js line 143 if (!$.hasClass(p, ID + '-wrapper')) {. Maybe that code is specifically looking for splide-wrapper so won't like splide-wrapper--nav. Anyway I'm sure you will understand it much better than I do.

Here's the template I ended up with. I skip the splide-wrapper class, but keep the others. I also added a class based on the optionset which is handy.

{#
/**
 * @file
 * Default theme implementation for a splide wrapper.
 *
 * Available variables:
 * - items: A list of items containing main and thumbnail of splide.html.twig
 *   which can be re-position using option Thumbnail position.
 * - attributes: HTML attributes to be applied to the list.
 * - settings: An array containing the given settings.
 *
 * @ingroup themeable
 */
#}
{%
  set classes = [
    settings.skin ? 'splide-wrapper--' ~ settings.skin|clean_class,
    settings.skin_nav ? 'splide-wrapper--' ~ settings.skin_nav|clean_class,
    settings.vertical ? 'is-wrapper-vertical',
    settings.vertical_nav ? 'is-wrapper-vertical--nav',
    'splide-wrapper--' ~ settings.optionset|clean_class,
    blazies.is.nav ? 'is-wrapper-nav',
    settings.navpos ? 'is-wrapper-nav--' ~ settings.navpos|clean_class,
    'over' in settings.navpos ? 'is-wrapper-nav--overlay',
    'over' in settings.navpos ? 'is-wrapper-nav--' ~ settings.navpos|replace({ 'over-' : '' })
  ]
%}
{%- if blazies.is.nav -%}
  {% set classes = classes|merge(['splide-wrapper']) %}
{%- endif -%}

<div{{ attributes.addClass(classes)|without('id') }}>
  {{- items -}}
</div>

In #4 I suggested to leave the code path for the old mailer almost entirely untouched, and create a new service only for the new mailer. If we make a new service in this issue, then it's not experimental. It might not be what we want for the new mailer. Because of this uncertainty I suggested postponing this issue for a few weeks at least.

I'm unsure where the notion of of custom $ops comes from exactly, do we have any data that supports that this is a thing people did?

The user_registrationpassword module "sort of" makes up some new $op values. However from a quick inspection it has separate config, _user_registrationpassword_mail_notify() and _user_registrationpassword_mail(). So it likely isn't actually an example of this "thing", but it gives a hint why people might try it.

The MR here turns the $op argument into an internal implementation detail. It isn't API anymore and therefore may remain a string.

Please see #6. $op is definitely an API as any code that alters emails depends on the values. Also configuration depends on the values and the code in AccountSettingsForm must match. True, it's already a string and I agree it isn't necessarily correct to fix that by a widespread replacement everywhere in the code. Still, I feel that creation of separate API functions that make the $op appear more internal isn't necessarily helpful. It makes the interface complex and creates dull boilerplate copy/paste implementations. Could we instead create an enum on the interface??

I removed the BC for custom $op parameters.

The comment for _user_mail_notify() clearly states the allowed values for $op, so I agree we don't need to support any others.

One thing to note is that this also changes the return value.

Of course as @berdir says, _user_mail_notify is technically an internal function. We have decided that sites treat it as an API (what else can they do?) so we will do the same. For an API, a return code change is a BC break, so I feel let's leave it unchanged. We can still have our preferred return code on the new interface. We could make sendToUser() a public @internal function for BC only, which simplifies _user_mail_notify(), removing the switch statement.

Thanks @danrod you are right.

I see it now: libraries/splidejs--splide/dist/css/splide.min.css (that's the path on my system anyway). My mistake was that I was looking for the file within the module not the library. I acknowledge that the explanation did include the word "library" - I just didn't get the meaning😃.

znerol → credited adamps → .

adamps → created an issue.

adamps → created an issue.

It looks like we could do something like this

if (function_exists('update_requirements')) {
  $requirements = update_requirements('runtime');
}
else {
  $requirements = \Drupal::moduleHandler()->invoke('update', 'runtime_requirements');
}

My sites are still on D10 so I can't test it.

All patches go into 2.x please, which is the default branch.

I believe this is "working as designed" and the behaviour is the same in swiftmailer and Drupal Symfony Mailer Lite.

If you pass in MarkupInterface, then it is treated as HTML. You can control line breaks using HTML tags such as br and p. Sure, Drupal can generate the correct line endings, but they don't have any affect on the layout of HTML.

I guess there must be a bug in your config but there's not enough info to say.

Sorry this issue queue isn't for questions like that, it's not a helpline. Try https://drupal.stackexchange.com/

"Import" converts the current site config to the new format, which is quite a different thing from a default. If you want the defaults then you should do "enable" (without importing), or "reset". All of that is part of the "mailer override" module, which is for optionally replacing the implementations of legacy mail implementations from other modules (so yes it's non-standard, it was tricky).

If imported like the User module then you lose all of your changes if you import them again.

True, and it gives a clear warning. How else could it be? I am not a magician😃.

===

If you are writing a new mailer, then ignore all the above. You can just provide a straightforward default in config/install directory. Then you could compare/reset it in the usual way with the config update module. This seems to be exactly inline with how config works in other parts of Drupal. Does that answer your question?

You could also hardcode a value into your code. It could still be overridden in the UI, but the UI wouldn't see the value from code. So maybe this what you tried that wasn't working?

The problem is that Core doesn't provide an API for the information that we need. So I had to call an internal function, which has now been removed. We can check the issue that removed it to see if it explains the replacement to use.

Good idea. I like ✨ Execution environment: Safely run code inside an environment with partially substituted system state / config Active . As you say, it decouples negotiation from switching. We can use it in the new mailer in ✨ Create a new mailer service to wrap symfony mailer Active or a similar issue. If we wish to back-apply it to the old mailer then we can re-open this issue.

We should close some old plan/meta issues or relate them if they're still relevant.

I feel we do currently have a problem with multiplying and semi-duplicating issues. I raised a bunch of issues a year or two ago. Some of them are a little out of date, but still they have many interesting ideas, and also some valuable discussions with @berdir and other core maintainers.

@znerol has since raised his own issues. They tend to operate in a different "space" without referencing the prior ones, often using different terminology. Some of them might now be outdated, others still contain important ideas or MR.

I feel that it would help now to merge into a single set of issues referenced from this IS that we all use moving forward. We can transfer information in from others and then close them as duplicates. Once the group has agreed that an issue is in scope, then we can open it up for coding and allow different MR to be compared side-by-side.

As per #29, @zengenuity and @adamps will aim to create a design and propose how it could map to issues, bearing in mind @berdir feedback. Then we can get review from the group, and if we manage to get agreement someone can do all the issue management.

I'm not sure how important discoverability and reusability of mail "templates" really is, it might in some cases even be undesired. We recently discussed that ECA explicitly doesn't support those special user password reset mails, to not allow to send them out in the wrong context, that could even be a security issue.

Interesting point. We definitely do need the discoverability as modules like DSM+ "Mailer Policy" need to know there is a password reset email so they can show a form to configure the subject and body. Removing the discoverability won't help security anyway: we'd end up instead with a form that allows the admin to type in the ID of the tag/key (so they type password_reset), and it would be equally insecure. Or the admin could apply the hijacking context setting globally to all user emails, or even to all emails. Or the admin could enable an email logging module and read the logs, or they could configure a transport that sends emails to their own private server, or enable a module that redirects all emails (normally used on a test site) to a different address.

During the meeting we added a new requirement C3 to cover this case, which I imagine would work a bit like this. The metadata for each email sending component can optionally include a setting to indicate a specific email category/tag is sensitive. The Email can have a corresponding isSensitive() function. We can create a Core permission "view sensitive emails". The functions that would cause a problem including setBcc(), setTo(), setCc(), getBody() can then all perform a check on the permission if the email is sensitive. I guess we need a setting for code to disable the check in case of trusted code that isn't related to a UI. We don't necessarily need all this for a beta however.

I'd avoid creating too many issues to discuss and implement every single bit. It's hard to get things into core piecemeal.

OK thanks for the advice. Still, I feel we need a way to take some decisions individually (and sure we could change our minds later), rather than evaluating the whole thing in a single MR.

I see that DSM has the concept of mail params on the object. That's still keys and not types that can be enforced, but I think that's an improvement over the single magic mixed type that this has. With mulitple params, we can still document them and their type.

Based on feedback from 1.x, now in v2.x we have a clear interface such as UserMailerInterface which has types for each function argument. These then get converted into the type-less array params, but we have at least ensured they are the right type in the first place. We also have a function setEntityParam(EntityInterface $entity) which automatically sets the param with a key matching the entity type, so it gives some level of type hinting. We could even add add code that prevents overwriting such a param with the wrong type.

In addition to the initial params set at the call site, we found that hook code sometimes just wants to record some data into params (perhaps to read again at a later stage in the email pipeline). Forms have setValue(), rendering allows adding anything to $variables. So probably we can't reasonably try to control everything that goes into params.

IMHO, the hardest thing of the whole process is experimental code/module and BC in all kinds of directions.

Yes you are right, we should have some requirements about that.

They'll still work, but sites will need to be aware to configure their mail transports in two places and so on.

That sounds scary to get working correctly, to test and support. Maybe we should allow only one active mail system on a site?? Even so it will be plenty hard enough.

I think we should have a single flag that switches all (supported) mails to the new API.

I was thinking the same thing. Let's keep it simple.

Don't think we want or can automatically transform hook_mail() implementations, there are too many custom things going on there with attachments, HTML and what not.

Actually it is definitely possible without too much complexity, we have it in DSM+. Not necessarily guaranteed to work 100%, but in practice we haven't really had new issues recently. Perhaps it doesn't need to be in Core - sites could choose to enable it from Contrib if they want to work that way.

So perhaps the requirements overall are something like this:

1. The experimental mailer reports which modules support which mail systems (some might support only one, some both).
2. The experimental mailer module provides a setting to choose which mail system is active: old or new. All modules use the active mail system. (We could skip this page at first, and use enabling the experimental mailer as the switch. However it's arguably a more realistic beta if we have the page, and it allows the user to see the level of module support before switching)
3. By default, emails are dropped if the module doesn't support the active mail system. However there is a mechanism to enable a Contrib module to perform conversion.

I feel that separate functions would likely just be a nuisance in this case as the implementations are all identical (except 'register_pending_approval' generates the extra mail, but that can be a simple if test like now). The values of $op aren't really internal because they form part of the email "ID" - module/key in old terms, tag or whatever we might call it. This will be referenced in the config/code of any module that is involved in building or altering user mails. The list of $op values, and their labels is needed for the form UI that controls such config.

Sorry cross-post removed a related issue - now fixed

Definitely +1 for the idea in general - I agree we should end up with a user mail handler service.

I'm not sure if we need this issue specifically for user module however. As @berdir pointed out in the roadmap, every module will need a migration from old mail handler to new. Likely during the transition, the new mail handler will call the old one whenever the site uses the old mail system; the old mail handler is marked deprecated then later removed at the same time as the old mail system. (Contrib could instead use a major version change, the old branch for the old mailer and the new branch for new mailer.)

The new mail handler service could often want a different interface from the old, so we might be better to create a new service name rather than reusing the old one. We might also wish to specifically identify these new services, perhaps with a tag (which would allow checking which modules support the new mailer - perhaps a neater alternative to @berdir idea of #[SupportsMailer] on hook_mail()). Perhaps they share a base interface. Perhaps they share a way of yielding metadata. In DSM+ we have UserMailer as both a service and a plugin, and I feel it works quite well - it's an option we can consider.

Although the old mail handler for most modules is a service, the old mail handler for user is _user_mail_notify(). However it doesn't necessarily make much difference, and we can use the same migration mechanism.

In other words, we don't necessarily need a migration/deprecation step separately in this issue when we are already going to have one anyway for introducing the new mail system. I feel that it might be worth waiting until we have a clearer idea of the overall mailer design, especially the migration part, before we work on this issue.

I feel that we should have a group decision process for agreeing which issues are actionable. At very least it could be like setting RTBC status - please don't put your own issue in, at least one other person should be involved😃. FWIW I would put this issue in the "Mail building Components" section in the roadmap.

Personally speaking, I definitely agree with the idea. We are currently having a little discussion in the issue about how to do it, then once we have agreement at that level it becomes actionable.

I copied the overall design in from the meeting notes and tidied it up slightly. Next step @zengenuity and @adamps to develop ready for review.

Older version saved for reference

Proposed resolution

Introduce a new plugin type and a config entity type which work in tandem. I.e., use plugins to realize a strategy pattern (substitutable implementations) and the config entity to select one of them for each case. The combination of plugin type and config entity type is a known pattern in Drupal core. Examples for this architecture are Action, Block and Editor.

A sensible name for the plugin type and config entity in the mailer realm could be EmailTemplate.

The parts should roughly fit together as follows:

• Each implementation (email template plugin) declares which type of email it is capable of rendering. The responsibility of an email template plugin is to turn input data of a specified type into an email data object.
• Each call site (email sending code) declares the type of email and its specific variation it wishes to dispatch.
• A config (email template entity) links the email type+variation combination to a specific template implementation.

Call site

The call site (email sending code) interacts with the email template config entity. Config entities are easy to deal with in both procedural and object oriented code. The render() call is forwarded to the plugin implementation.

Procedural example (error handling removed):

$account = User::load(1);
$template = EmailTemplate::load('user.password_reset');
$email = $template->render($account, $account->getPreferredLangcode());
$mailer->send($email->to($account->getEmail()));

EmailTemplate config entity

Config entities can track their dependencies. Config which is shipped in a config/optional directory is checked whenever a module is enabled or uninstalled. Optional config is installed as soon as all of its dependencies are met and removed whenever one of its dependencies breaks. It follows that the experimental mailer module can safely provide config for all mail sending core modules. Whenever the experimental mailer module is enabled, the subset of email template config entities having all dependencies met is installed. Whenever the experimental mailer module is disabled, all the email template config entities are removed and the system is config wise back to the point before enabling the mailer module.

Contrib and custom code can list the email template config entities. This represents an exhaustive list of all transactional mails potentially sent out from a site.

EmailTemplate plugin

Core may initially ship rudimentary mail template plugins mirroring the current behavior of core hook_mail implementations. Better implementations (e.g. based on entity view modes) can be added later in the development process.

Contrib mail sending modules (e.g., Commerce, ECA, Simplenews, Webform) can add their own mail template plugins and config entities for their own mail types.

Contrib mail building modules (e.g., DSM+, Easy Email) can ship additional mail template plugins for existing mail types and substitute the default implementations.

Coming out of discussions at our last meeting I have been contemplating how we can progress efficiently with group agreement.

I feel that we will move forward by making individual decisions. Each one is a specific choice in a single localised area from a defined set of options. First we make a choice regarding the existence of an EmailInterface, which if we agree, leads to an issue. Then within that issue we have separate choices about each of the fields that could be on it, leading to an actionable issue that can be coded (the coder can make any more detailed choices themselves). Probably we will make 100+ of these choices together as a group. It seems a lot, however many of them can be done in 2 minutes. I felt that @zengenuity mostly agrees (please correct me if I'm wrong).

Lorenz says that he likes to think in code. I feel it would work well to make prototypes within one of the issues that we agreed as a group. We could have an issue for creating the component mail building code, and we could agree that for each idea someone had they would create a MR for only user module - maybe just 50 or 100 LOC. The author could explain the key points and advantages in a comment.

However prototypes for the large parts of the system (500-1000 LOC) from my point of view are not good for making decisions because it is like making 50 choices at once, and it's too much to take in. Potentially they generate multiple issues = noise in the issue queues, sometimes duplicating other issues, and drawing the attention of outside reviewers to the prototype rather than the decisions. Personally speaking I wonder if this kind of prototyping might even be better on a personal gitlab clone or similar??

How does anyone else feel?

Coming out of discussions at our last meeting I have been contemplating how we can progress efficiently with group agreement.

I feel that we will move forward by making individual decisions. Each one is a specific choice in a single localised area from a defined set of options. First we make a choice regarding the existence of an EmailInterface, which if we agree, leads to an issue. Then within that issue we have separate choices about each of the fields that could be on it, leading to an actionable issue that can be coded (the coder can make any more detailed choices themselves). Probably we will make 100+ of these choices together as a group. It seems a lot, however many of them can be done in 2 minutes. I felt that @zengenuity mostly agrees (please correct me if I'm wrong).

Lorenz says that he likes to think in code. I feel it would work well to make prototypes within one of the issues that we agreed as a group. We could have an issue for creating the component mail building code, and we could agree that for each idea someone had they would create a MR for only user module - maybe just 50 or 100 LOC. The author could explain the key points and advantages in a comment.

However prototypes for the large parts of the system (500-1000 LOC) from my point of view are not good for making decisions because it is like making 50 choices at once, and it's too much to take in. Potentially they generate multiple issues = noise in the issue queues, sometimes duplicating other issues, and drawing the attention of outside reviewers to the prototype rather than the decisions. Personally speaking I wonder if this kind of prototyping might even be better on a personal gitlab clone or similar??

How does anyone else feel?

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.

I started developing DSM+ 4 years ago to meet very similar same user stories and requirements. Through the experience of 46k sites we fixed 313 issues. Various other Contrib modules and likely numerous sites have written code to integrate with the interfaces so if we can keep those interfaces it would simplify migration of these sites to Core. Recently the 2.x branch is a significant rewrite based on all the things we learnt in 1.x. I agree we shouldn't blindly copy it into Core, but also we shouldn't blindly ignore it. From my perspective, before prototyping a new system we should look at the existing one and see what we feel needs to change and why.

Certainly @znerol has some ingenious ideas which we can also include in our overall solution. Also I see some things that I tried 4 years ago and didn't seem to work out so well; I see some key points that I learnt through direct user feedback that he likely would also need to consider. This is entirely natural as DSM+ is a mature codebase proven in the field in comparison with a prototype developed over a few weeks (or maybe months) by one talented developer.

Thanks @berdir

I don't think LOC is the primary factor to consider for this. Also because the current examples are way more complicated than they need to be due to feature flags, the experimental module and BC. The actual implementation would be a lot less.

I accept all that and I certainly don't believe we should be trying to minimise lines of code in an artificial way. I mentioned LOC just as an illustration - I could equally well present a list of detailed specific points to demonstrate my point about complexity.

But overall I'd clearly recommend keeping the actual mail builder interface/API as plain and un-opinionated as possible and start with that.

I certainly agree with that and it's exactly what I was trying to do with #19.