🇬🇧United Kingdom @adamps

Has anyone found a way to implement HTML emails correctly using Symfony Mailer (DMS+) for Drupal?

Yes very many people have.

Only a small number of people are having a problem. There isn't yet enough information here to reproduced the problem. Normally it works, there must be something unusual that stops it working for some sites.

/usr/sbin/sendmail -t should likely be the default instead of -bs

Please see #2. Seeing as symfony don't recommend -t I doubt we will add it here. Also note that the same code is now in the experimental Core mailer, and we would like both to remain consistent.

My second point is an attempt to respond more directly to what you have done here.

I'm mostly confused about the problem/motivation in the IS😃.
1) I believe that we are not currently updating the validity field when the confirmation period passes.
2) According to my understanding of the cancellation issue, neither type of cancellation has a validity state. It's simply a restriction of dates, which could include back-dating the cancellation so that it was cancelled on the day it started.
3) I agree with your first sentence, and I don't believe we plan to make such an assumption.

And I don't understand many of the changes in the MR. It's not clear to me how they relate to the points in the IS. The code is as it is for a reason, and those changes seem to break things. I'm not attracted to the idea that interface concepts are different from the underlying state - especially isValid(). I don't like the idea that invalid is different from a negation of valid, as I feel that's confusing. I don't really see a need for confirmed date in addition to a claimable date - already there are about 6 dates, and it's quite complex.

I guess I don't really get what you are trying to do here, sorry.

My first point is regarding issues and features. Where possible I feel that we should generate MR related to features with a defined spec in the IS.

We have 🌱 The problem of changes Active for cancellation which refers to 📌 Figure out contexts and donors Active for a very long detailed discussion. The current cancellation code in 1.x branch is not especially meaningful or correct, as it predates many important discussions. If you are not happy with the results of the previous discussion then I would suggest to continue it in the other issues although potentially we will never finish this project if we keep changing our mind😃. It seems that you are now using "retract" in a different way than you did in the IS of the other issue.

I feel that cancellation is an important, big, complex feature. I wouldn't want to mix it with anything else, I wouldn't want to do it in parts, I would do the other issue in entirety when we reach the appropriate point.

Then in the roadmap we have a bullet like this:

Written statement. There is a field for Staff to mark a written statement as sent on a specific date. There isn't any mechanism so send one which could be via email or generating text for a letter to print or save as PDF. There isn't any further audit information such as which template version was used (could be a config entity).

It would make sense to me to turn this into an issue which again I would class as a feature request. We could have a discussion on that issue about how to do it.

adamps → created an issue.

The proposed resolution covers only some of the acceptance criteria. Others parts are covered well in some existing issues:

• ✨ 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

I like the idea of a plug-in and a config entity.

I suggest that the plug-in needs full control of the email building process - including to, langcode, etc. So it would be

$template->send($account);
// OR
$email = $template->render($account);
$mailer->send($email);
// OR
$template->render($account)->send();

The advantages are: all the code is in one place; there might be multiple call sites, and we don't want to duplicate logic to each; code that overrides the plug-in or alters the emails can override/alter all the parts - otherwise the to address and langcode are effectively locked; also the to address may depend on language (e.g. send to the site address and site name, the latter being translatable) so it should be evaluated after the language switch.

Other email template types would have different parameters. Contact messages would need $template->send(MessageInterface $message). Simplenews needs 2 params, the issue and the subscriber. This suggests that we need an interface that informs the calling code what parameters to pass - there was clear demand for this in DSM+, and on other issues linked from the META. The interface is identical for all user sub-types and the implementation likely is too, but the config of course is different.

The mechanism of EmailTemplate::render() cannot support this variation in the interface (it can accept arbitrary extra parameters, but it cannot support type information => IDE code completion). This suggests instead the possibility that the plug-in is also a service, which allows all the flexibility of auto-wiring, dependency injection, auto-configuration (to set the plug-in manager automatically to be the factory for the service) etc. We can put the burden of loading the config entity into the framework code rather than requiring each call site to do it - which is important as I believe there won't be universal agreement on the configuration structure: Core will have a basic one and Config may need something more powerful. So the call site code can look like this:

__construct(protected readonly UserMailerInterface $userMailer){}
$this->userMailer->send('password_reset', $account);

or

Drupal::service(UserMailerInterface::class)->send('password_reset', $account);

Note that the interface is the same as the existing _user_mail_notify(), which seems like a sign we are on the right track, and should help during the transition. Once the new mailer is no longer experimental, we could alter all call sites to call Drupal::service(UserMailerInterface::class)->send. Then the new mailer plug-in could check if the site has chosen to enable the new mailer service yet, and if not instead call _user_mail_notify().

I guess there are two possible options: have a base field, and hide it when it's not needed, or add a bundle field whenever it's needed. For the 2 web self-declare cases, the explanation text isn't in the evidence, so we would need the bundle field.

- the explanation could actually be on the confirmation email, so it could be missing initially

I had also considered that we could track the confirmation email wording or template version. I recall that the guidelines said something about "demonstrating that the email had been sent according to a particular template" being acceptable.

Maybe best to make this a seperate issue? I can do it.

Thanks I raised 📌 Fix FunctionalJavascript test Active .

adamps → created an issue.

I ended up deleting the last two items:

4) Fix deprecation notices in test results (although some come from Commerce tests?)

CheckoutTest has deprecations, but AFAICS they come from Commerce.

Remaining self deprecation notices (2)

2x: Renderer::renderPlain() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Instead, you should use ::renderInIsolation(). See https://www.drupal.org/node/3407994
    2x in CheckoutTest::testCollectDeclaration from Drupal\Tests\gift_aid_commerce\Functional

Other deprecation notices (12)

  12x: Using the 'timestamp' formatter plugin  without the 'tooltip' and 'time_diff' settings is deprecated in drupal:10.1.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/2993639
    12x in CheckoutTest::testCollectDeclaration from Drupal\Tests\gift_aid_commerce\Functional

5) Move the menus one level down, e.g. put Gift Aid under Services? This is the normal level, and it means that the various admin pages for the module are all visible as tabs.

Things have changed since I wrote that originally. Each of the admin pages now has its own tabs, so it no longer applies.

This is presumably a beta-blocker, so I'll comment on it to bump it up the list.

This seems essential - it's a bit that got left out of ✨ Fix GiftAidOverviewController Active

Without this, anonymous declarations will fail to arrive on the user, so I've altered the issue settings.

I'm happy for you to say it's not a road-map priority. Even so, I feel we should have these clearer issue settings to help us or anyone else see the status in the future.

We've done most of these now, the rest likely aren't important.

Thanks. I will make the fixes in 📌 Tidy up Active

I kind of like having submodules for atomic features

Fine. Potentially it could use some documentation so people can find it and a test so that it has a chance of working when they do😃.

Now have a triple green on the test results😃. OK with you?

I have never installed the necessary for infrastructure for FunctionalJavascript tests. Probably we need just one test, so it's not really efficient for me to learn it. Would you be willing to do it yourself please?

In DeclarationFormTest::testOralDeclarationRecordedByStaff(), the return statement should be down to just before @todo Fix when added code for cancellation.. It just needs some ajax to add evidence, which might start something like

$this->assertSession()->waitForElementVisible('named', ['button', 'Add new Gift Aid evidence'])->press();

I'm keen to do some basic tidy up at this point for clarity and efficiency moving forward. I'm interested in the first 3 please. What do you think?

adamps → created an issue.

but it breaks the links.task

Exactly.

The tests now pass again, so let's get this in.

The changes in gift_aid_user.routing that I was surprised that they work - well as far as I can see, they don't😃.

The gift aid tab on the user was missing, which I fixed by correcting the filename to gift_aid_user.links.task.yml. However then it hits a bug with missing parameter context, because the parameter is inherited from the user account, which is user.

The fix was quite difficult. For the ContextOverviewController::view() method I discovered that I could get passed the whole request. For the access check, the same technique didn't work however I found that I could implement my own access-checking service.

I've also adjusted the forms as explained in the comments on the MR. The form is now called DonorContextForm, and is used whenever editing/adding a Donor from within a context.

adamps → made their first commit to this issue’s fork.

Here are the old requirements from the original IS. They are mostly outdated by the new IS, however they are kept here because they add some extra detail.

Requirement: extra data associated with each email

• Tag/category that identifies the emails, corresponding to module/key in Core.
• Parameters that define the email to build, according to its tag/category. Matches params in Core.
• The language for the email = langcode in Core.
• Theme to use for templates and CSS. Can be set globally in mailsystem. Adding it here is more flexible and simpler (all the data is available on a single interface).
• User account. Set in Contrib module simplenews, and relevant when rendering entities. For security, recommend switching to anonymous if no specific user is requested to ensure that the access of the current admin user is not used when sending to an ordinary user.
• Transport. Can be set per-category in mailsystem. Add it here for same reason as theme.
• CSS libraries to attach?? Hard-coded in DSM-L. DSM+ allows modules to add libraries with their own email CSS. We could instead get them to add CSS to the single hard-coded library.

These can be get/set by any events/processors and later events can override the value set earlier. Tag is supplied when creating an email and is read-only after that. Language, theme and account are read-write during an initialisation phase, then are read-only once email building begins.

Requirement: building in a callback/event

The code that sends an email should specify just the tag and params. The actual building then occurs in a callback or event. Other code can register events/callbacks to alter the email, or could even replace the entire email building code. This corresponds to hook_mail() and hook_mail_alter() on the existing mail system, and the reasons for doing it are the same as before.

1. Customisability: the code that alters the email has exactly the same information available as the original building code. If the building code outputs a fully-built email directly, then it can be difficult or impossible to reverse-engineer the parameters.
2. Context switching: after the initialisation phase, a context switch occurs of 4 parts: render context (to avoid leaking into current request), language, theme, and user account (for access control and correct rendering of entities). Render switching occurs already in Core and theme switching is done by mailsystem. Language switching occurs in Commerce and is highly advantageous to avoid bugs where the caller forgets to check the langcode. It would allow removal of half the code in user_mail(). User switching occurs in simplenews.

Requirement: flexible and customisable HTML building

The requirement is the same as when building an HTML web-page. The body should be built in a "structured" format (rather than just flat HTML), containing placeholders for values that can be substituted. Later events can therefore alter just one part of the email, and can use placeholders in their replacement HTML. Code can also alter the values of the placeholders. We would expect to re-use the mechanisms we already have: render arrays, TWIG templates/variables, and tokens.

This roughly corresponds to body on Core MailInterface, which is an array that contains a mix of HTML and plain text strings.

Some other fields should support placeholders that can be substituted, especially subject and plain-text body.

Requirement: Extra steps in the email building process

Apart from the ones that require API changes, these could be moved to Contrib. However in some cases the mail system would hardly be useable.

1. Attachment access checking (some done in DSM-L). Important for security, because emailing an attachment has the potential to bypass normal access-checking on the file. This is not just theoretical as there was already a security issue in a popular contrib module. Requires an API.
2. Set some defaults (currently in MailManager)
3. Convert relative URLs to absolute (currently in MailManager)
4. Ensure that subject is plain text (currently in MailManager) - requires a different API from symfony function subject(string)
5. Convert HTML body to plain-text (currently done in DSM-L) - Symfony will do it, but in quite a different way from Drupal
6. Inline CSS (currently done in DSM-L)

Requirement: extensive customisability

1. Allow replacing every part of the Core email building process. This could be achieved by using services, plug-ins, and events (see next bullet).
2. Support flexible registration of callbacks/events. Code can specify the precise timing (for example by means of weights) and the scope (option to restrict to specific tags). Allow multiple registration for different timings/scope. Allow custom code to remove the registrations of other events.

adamps → created an issue.

adamps → created an issue.

I take your point, but I still lean in favour of declaration types.

OK no problem.

Assigned to J to document ideas for "Alter some declaration fields, converting multi-state values to multiple booleans"

In #3 I wrote

In various ways you would like the evidence types to influence fields that are actually present on the declaration.

We realised that even if we change to declaration types, then we still can't set a per-bundle default to base fields. Instead we would need to add properties on the declaration type, with logic (perhaps on pre-create) that uses those properties to set field defaults.

It just occurred to me, if we are doing that, we could equally well put the properties on the evidence type. So to revisit the TLDR of #4, we need 2 bits of "funky work", whether or not we have declaration types.

1) A mechanism for selecting bundle type (applies to either/both of evidence/declaration) - either a new page, or an enhanced select widget. We would prefer to show descriptions as well as labels. We might like to filter out some values (already we filter out the web ones).

2) A mechanism for setting default declaration field values based on the bundle type (either evidence or declaration).

In addition there are some further work items

3) Add declaration types

4) Alter some declaration fields, converting multi-state values to multiple booleans

Looks good to me, thanks @znerol. I will leave in NR until @berdir checks.

You can ignore my previous remark as we can change the name as part of ✨ Framework for testing Symfony Emails Active (I suggest MailerTestTrait).

Thanks for the updates (not yet checked). Personally I prefer the existing name of AssertMailTrait or similar. It would then be natural to add extra functions with specific asserts in ✨ Framework for testing Symfony Emails Active .

Thank you for your contribution. I'm afraid that I'm not going to respond right now. Contrib module maintainers do their work for free in their spare time, and the availability of time comes and it goes. Sometimes I have waited for over a year without any response on other modules. Even when the maintainer doesn't yet answer, the other issue followers might appreciate your work.😃😃

I made a first draft to start the discussion.

I think it's strange that we then use a trait from a test module as an API. Because this is an API.

Is there any harm in just switching to collecting by default?

+1

There are two main classes in this issue: the capture transport, and the API for accessing captured emails. We can put these in the natural location as @Berdir suggests - not inside a test module or anything.

Then we just need a mechanism for enabling the capture transport. It's much less important where this is located because the exact mechanism is internal. It could be a test module, however I feel there is likely a better option. My impression is that the experimental mail plugin is mostly just a proof of concept / placeholder, and maybe we could even delete it when the new mailer lands. Even if we don't, then could we keep testing of the experimental module separate (a minority case) from normal testing, e.g. in with a separate base class hence provide a separate dsn?

We are in agreement that the declaration can have multiple evidence, but it's an edge case.

The declaration add form wouldn't even need to show the evidence field.

OK that removes many of my concerns. Still, this is a fair chunk of work. We need to add the declaration types (which we had the code before, so just finding it), create some default entity config and write code for the "add declaration" page for choosing the type. We need to move a few fields around, and adjust both pieces of code that generate web based declarations and adjust the corresponding tests.

as discussed in #3534245: Change handling of web evidence it's a nice way to generically store audit information about a web submission, such as the explanation given

Surely if we are having declaration types, then we no longer need web evidence?? We would have web declaration types instead. I don't see how the "supplementary evidence" on a declaration can ever really be web (self-declared) - it would always be staff.

Good point. This seems quite important to me in general. I can see it's less important specifically for a site where the orders are almost all placed by authenticated users.

Thanks.

My strongest feeling is a concern about putting the User entity on the evidence. In various cases it could end up hanging around pointing to the wrong user:

• 📌 Donor merging Active
• 📌 Update donor context if order assigned Active
• Manual editing by staff to fix incorrect assignment of donor context to a user.

The first two are handled by code so I guess we could make the code change both places. But still, it seems to be creating trouble and work for no clear benefit. If you would like to display a link to the user on the evidence view, then I'm very happy with that - in effect it is a calculated field.

For the URL and label, with the current design both are redundant information. The label is equal to the description on the evidence type. The URL could be determined. So my feeling is similar to above - both of these are in effect calculated fields, and we don't need to store the value in the database. However I'm less concerned about doing so because there isn't a clear way for the information to become outdated.

1) Originally I had explanations on evidence. I moved it because evidence is also used for cancellation and CoA which don't need explanations. If we put it back then we need to handle that.

2) Please can you explain why? The term "Web" is currently used as the method for all self-declarations. To use it again for a specific one of them could be confusing.
- URL: currently the URL is entirely determined from the User. This isonly needed if we use one evidence type for multiple scenarios, which I don't understand the advantage of.
- User: already present via the reference to Donor, but I guess we could put it again.
- See 1). But if we do add it, then I suggest put it as a base field because it will apply to all the evidence types.
- Label of what?

3) Sure, we could, but I don't really understand why. It seems to be entirely duplication of information, which I'm not especially fond of (what if staff edits one place but not the other?).

I have to say that this idea rather blows my mind😃. It feels like each "use case" or "scenario" ends up with both a declaration type and an evidence type. Some concepts/fields already on evidence type are duplicated onto declaration type. Sometimes the declaration/evidence will only make sense in the particular pairings, other times it could be more flexible but at least an oral declaration should have an oral evidence type.

I feel that your IS is written from the perspective that the "no declaration type" option is hard, however I'd like to explore the problems you present:

1. Not necessarily "quite a few fields" but one field about allowable use cases (declare/cancel/CoA) should be enough. Is it hard to understand? For each specific paper form there would be an evidence type. Seeing as the forms are reasonably examples of evidence then it isn't necessarily so bad. Telephone and in person seem to work well. The evidence types could be re-used for change of address or cancellation. I feel it's harder to understand the "declaration types and evidence types" system, because the poor admin has to create both entities. And some of the evidence types could end up anyway being specific to declarations only, so it's almost like the worst of both worlds.

2) Yes there would need to be some restrictions. But it's likely not as bad as the restrictions needed if we have both declaration and evidence types.

3) Declarations would generally have just one evidence I expect. HMRC are clear I believe - any change should be a new declaration. Editing is only allowed for corrections.

I see a real problem with your scenario different from the above 3. In various ways you would like the evidence types to influence fields that are actually present on the declaration.

1. In case of the paper forms, you would like to hard-code a value for explanation (set a default and hide the field). The evidence type currently doesn't allow this because that field is on declaration.
2. You would like the evidence type to influence whether confirmation is required. Actually it can already via the method field. Web = no, oral = yes, written = maybe. But perhaps you'd like more flexibility: a written format that definitely requires confirmation or any staff format that definitely doesn't etc. This has a fairly easy solution: we could change the method field to "written confirmation required" with values yes/no/maybe.
3. You would like the evidence type to influence validity and start date - which are declaration fields.

If we are going to have declaration types then potentially we should remove the evidence entity, or at least stop using it for declarations. This is going to be a fairly large piece of work.

OK that makes sense.

Is it OK with you if I add ✨ Framework for testing Symfony Emails Active into the roadmap??

znerol → credited adamps → .

The module name is currently mailer_capture. This is good for the current use case.

In ✨ Framework for testing Symfony Emails Active I propose to add some extra functions that I believe will be very useful, see also #13 explaining the benefits. These functions would naturally live in the same module, probably even the same trait. Therefore I propose that we change the module name now to mailer_test, and the trait to MailerTestTrait. The module/trait are responsible for assisting testing of emails, which includes both capturing the email and asserting the values. Developers will almost always want both parts I believe, so we don't need a module for each.

How do you feel about that??

Good idea. I created 🌱 Requirements and strategy for Core Mailer Active .

The goal of this issue is to create specific and clearly defined issues that have majority/consensus support from the core group working on this area. Those issues can then be added to the road-map and assigned to individuals to work on.

I propose that all the other issues be postponed on this one. We don't have to actually set them to postponed, we just wouldn't work on them until the requirements/strategy discussion group has agreed that they are the right direction. Otherwise we risk having conflicts with different people heading in different directions.

This doesn't have to be a heavy/slow process. For example on 📌 [PP-1] Add a way to capture mails sent through the mailer.transport service during tests Postponed I have just one question, as per my comment #16, which could be resolved in 5 minutes.

Before this gets committed I feel we should discuss #13 please.

In preparation for ✨ Framework for testing Symfony Emails Active it would likely make sense to adjust this issue to have module mailer_test containing MailerTestTrait. Otherwise the follow on issue creates extra work for us and the core committers by renaming everything.

adamps → created an issue.

Please let's keep this issue a low-noise place.

OK. So where shall we have discussions about the details? If we can write that in this issue then anyone who wants more detail knows where to look.

Not fixing other existing bugs😃.

adamps → created an issue.

I've updated the IS. Hopefully nothing controversial, but if there are any issues then please let's discuss.

I expanded the Background/Motivation quite a lot, including more detail on the existing ecosystem and the importance of continuing to support all the existing features.

I kept the original emphasis on a simple Core making a fresh start, without any ties to existing Contrib. I did balance it by pointing out that it would help site builders if we matched Contrib whenever it did fit out needs, rather than creating a different API with different functions that effectively does exactly the same job.

I added a new emphasis on keeping existing Drupal features not in Symfony. In particular we rely on code living in a callback(hook_mail) and building the email based on params. There are 2 important reasons: context switching and customisability, which are both explained in detail in the specific issues described in the IS and #5. I realise that people are attracted to code like (new Email())->setTo()->setSubject()->setBody()->send(), but I have explored this in detail and I can't see how it can meet all the existing contrib needs. I have got as close to this as I can (it can still be a single line of code with the callback in an anonymous closure), although I found it more readable to create a mailer class with concrete functions for the callbacks.

Of course we can discuss this all further.

Great, this seems like a very thorough and precise treatment of these two headers - although I didn't check the standards myself to confirm correctness.

Message::getPreparedHeaders() has code that overlaps/conflicts with this issue (I'm looking at v6.4.17). It will set from based on sender so potentially we don't need setDefaultFrom(). And it will set sender based on from, which will undo the work of removeRedundantSender().

I'm adjusted the terminology in the title/IS slightly. Emails will pass through the transport layer, but they aren't sent directly to it as there are other layers on top (including delivery and likely another Drupal-specific one that we create).

I see this as a part of ✨ Create a new mailer service to wrap symfony mailer Active - it's one of the things that should be done as part of building every email. There is similar, but simpler code in MailManager. We could agree to create some Drupal-specific email events, in which case this should be adjusted to use them. So I feel that "postponed" is the correct status for a couple of weeks more.

@znerol How does that all sound to you?

Thanks that looks good. How do you feel about #13?

Below is a comparison of the code we could have in MailerCaptureTest before and after the trait added in ✨ Framework for testing Symfony Emails Active . The character count reduces from 559 to 264 = 53% reduction, and it is more readable IMO.

The trait itself is 300 lines, so not a big maintenance burden. Given that, I hope we might add it to core. If so, then it would potentially make sense to adjust this issue to have module mailer_test containing MailerTestTrait.

    // Before
    $capturedEmails = $this->getMails();
    $this->assertCount(0, $capturedEmails, 'The captured emails queue is empty.');

    $mailer->send($email);

    $capturedEmails = $this->getMails();
    $this->assertCount(1, $capturedEmails, 'One email was captured.');
    $this->assertEquals('admin@example.com', $capturedEmails[0]->getFrom());
    $this->assertEquals('State machine energy a production like service.', $capturedEmails[0]->getSubject());
    $this->assertStringContainsString('environmental along agree let', $capturedEmails[0]->getHtmlBody());

    // After
    $this->assertNoMail();

    $mailer->send($email);

    $this->readMail();
    $this->assertFrom('admin@example.com');
    $this->assertSubject('State machine energy a production like service.');
    $this->assertBodyContains('environmental along agree let');

Follow up from #9.

• It works if a Contrib module has overridden the service for TransportInterface.
• It works for Kernel and Functional tests

I like it - it has some clear improvements over the current DSM+. I have made an issue to change DSM+ to match this 📌 Alter mailer testing to match Core proposal Active . I'll wait until this is committed.

Also DSM+ has some clear improvements over this. We can use ✨ Framework for testing Symfony Emails Active if we wish to add them to Core.

I made some detailed comments in the MR.

adamps → created an issue.

I tried to integrate this patch into DSM+. I found that MailerCaptureTrait::getMails() currently returns SentMessage rather than Email as indicated by the comment. I fixed it as below, removing the reference to AbstractTransport.

class CaptureTransport implements TransportInterface {

  /**
   * {@inheritdoc}
   */
  public function send(RawMessage $message, ?Envelope $envelope = null): ?SentMessage {
    $capturedMails = \Drupal::keyValue('mailer_capture')->get('mails', []);
    $capturedMails[] = $message;
    \Drupal::keyValue('mailer_capture')->set('mails', $capturedMails);
  }

In the tests for this patch we should add some asserts on fields on the email, which would prevent errors like this from being introduced.

I think it's perfectly fine if applies() and the actual logic considers global context like current user/domain/configuration, you can just inject that in your service.

True, but in the case of DSM+ we have the theme stored on the Email class so we would need to have that available. That design is part of the mechanism to support the "Mailer Policy" UI that can configure many things including theme.

In the current form, I believe this patch would not be useable by DSM+. If anyone believes otherwise please explain. I believe it would be perfectly possible to alter the patch here so it would be useable, still providing the same features. If Core doesn't want that, then I can workaround it, because DSM+ can keep the current code, which is simple. It would mean having two mechanisms to set theme which could cause confusion to site-builders (imagine someone is changing a GUI setting but it has no effect), so it's not my personal preference, but it's entirely up to you.

The string "logo.png" is only in TestEmailBuilder.php. I think the problem is something local on your site.

Add configuration UI for the mail delivery layer.

The "full" solution is to have UI plug-ins corresponding to each Transport, which provide a form field for each parameter. There is code for this is in DSM+ and mostly copied into DSM-L. I had the idea that it could live in a separate project Mailer Transport → .

A simple solution is to have a setting to configure the DSN. However we found that GUI admins can find it difficult to understand the required escaping.

Low maintenance overhead while still providing the necessary extension points

This sounds like an excellent aim. From the perspective of DSM+, here are some priorities that I see. The referenced issues should provide a good checklist of points for discussion even if we end up making some different choices. If we can try to match DSM+ where possible it would make the transition easier for the sites already using it.

1. ✨ Create new mailer service based on symfony Active is key. Core can provide interface parameters for Contrib even if they aren't used in Core. With the old mail interface we had so many undocumented uses of $message['params'] that were inconsistent between modules, so it would be great to avoid that.
2. ✨ Create a new mailer service to wrap symfony mailer Active hopefully we can at least do some of these things, and match function that's in the old mail system. It seems fine if Core omits some features so long as the services/classes can be overridden by Contrib to add them in.
3. ✨ Events/callbacks for new mailer service Active important to have access to act on emails at all the relevant stages.
4. Policy: component mailer implementations should be swappable, likely as a service but could also be a plug-in. For example we create UserMailer service with function notify() that matches _user_mail_notify().

Some other issues:

• ✨ Migration strategy for moving to Symfony Mailer Active is obviously an important topic. I wrote some ideas, but it doesn't create any problems for DSM+ if it works a completely different way.
• Other issues like ✨ Create a plug-in system to build emails with Symfony Mailer Active , ✨ Create a system for configurable emails in Symfony Mailer Active are just ideas, no problem if they are left to Contrib.

Drupal Symfony Mailer Lite and Mailsystem are part of the old mail system - they are useful to indicate the required features, but the APIs are outdated.

Easy Mail and DSM+ (especially the "Mailer Policy" module), plus many others modules are users of the email API provided by Core. They indicate the required function and suggest what API could be useful.

DSM+ also contains a prototype for the Core mailer. For sure we don't have to copy it, but we can at least reference it for ideas of API and features, given that it is used in many sites.

This looks good.

It would be interesting to check if this still works in the case that a Contrib module has overridden the service for TransportInterface. It seems like it would, which is good news.

Which categories of testing does this work for (i.e. Unit, Kernel, Functional)? All 3 categories are likely to be useful.

There is a similar issue ✨ Framework for testing Symfony Emails Active , and good news is that the proposed solutions are similar. The other issue goes an extra step, proposing a full collection of assert functions for efficiently checking emails, for example: assertBodyContains, assertSubject, assertTo. The functions operate on the email at the front of the queue, and the function readMail() removes an email from the queue and then testing begins on the next one. This reduces the amount of test code substantially compared with each module having to navigate the email array directly. So the other issue can remain as a follow-up - or if Core doesn't want it, then it can remain in Contrib. See MailerTestTrait in DSM+.

adamps → created an issue. See original summary → .

Now available in 1.6.2 release.

Great thanks

There is now a second alpha as the first was missing this issue. Sorry I forgot to "git pull" after switching back to 1.x to add the tag.

Thanks. Done Alpha.

@Thanks ccarnnia. Please could you test MR176 as that is what I would like to commit?

After someone has tested and set it to RTBC then I will commit and make a new release.

I did the roadmap. Shall I create a first alpha release for you to test with?

OK to check this in? Then I'll update the roadmap, and I'm ready for you to have a look / test. After that I suggest we have another planning discussion.

Closed 📌 Finish the declaration forms Active as a duplicate of this. The only extra thing it had was the addition of the gift aid user tab, which I just added to the IS here.

Closed as a duplicate of ✨ Fix GiftAidOverviewController Active

I split out the original issue creating 2 new ones

✨ Events/callbacks for new mailer service Active
✨ Create a new mailer service to wrap symfony mailer Active

adamps → created an issue.

adamps → created an issue.