I honestly think that the current way routes are defined is really not great DX. [...] I think we shouldn't try to reuse terminology like defaults and controller for this. IMHO, it makes it harder to use, not easier.
This is good to know. I cannot really tell.
plugins are pretty much expected to have a class property in the definition.
True. In a previous version, the class
was set to the Symfony\Component\Mime\Email
. We do this for constraints, but I guess that this architecture isn't something we want to push more. It also required a custom factory - and I removed it again. The Symfony Email is a value object, it doesn't really map well to a plugin instance.
I think there is a way how plugins could have an interface for callers and still support dynamic arguments for the implementation. The methods which currently make out the controllers could be folded into plugin implementations. The argument resolver could be moved from the renderer to a shared plugin base. I guess I need to try that out next.
I do not think that mail is abandoned. At least, the mail.info.yml
declares compatibility with Drupal 10.
https://git.drupalcode.org/project/mail/-/blob/8.x-1.x/mail.info.yml?ref...
What about Core Mailer Preview?
Without rename:
Experimental Mailer:
Core Mailer Preview:
My intention was to only change the module namespace. But of course, if there are two modules available with the same human readable name, that would be confusing.
Thank you! I left one small suggestion.
I do not need help with that. Thank you.
Added a draft MR with an Password Argon2 module. This is implementing the idea from #32.
Thank you.
I was wondering why only the content block was converted and found the answer in the issue comments. Added that info to the issue summary.
Left a question and a suggestion in MR comments.
IS update.
@catch that would basically reverse the outcome from the security team triage call (#24). I'd rather try to go into a direction which doesn't send us running around in circles.
Agree with #43 regarding the UX. But I think we could mitigate that for most sites with #3530186-32: Introduce kernel parameters for password hashing algorithm and options → .
The idea there would be to add a new password_argon2
core module. People still have to manually enable a module, but this is likely a lower barrier than having to add the correct string somewhere in a yaml file.
larowlan → credited znerol → .
Fixed a misnamed variable. The approach in 📌 Extract _user_mail_notify() into a user MailHandler Active changed a little bit (internal service, no interface). Let's see how that goes over there and adapt here if necessary.
Opened an MR that renames mailer
into mailer_experiment
. In my eyes that name (a) underlines that the module will go away when the experiment is over and (b) it works as a feature flag.
(Although surprisingly with a setup like in #17 I'm finding I'm still logged-in after way more than 30s, will keep playing.)
You might be running into the edge case documented near the end of #17. If there is only activity in one tab from one user, then the session is not garbage collected.
My observations suggest that the behavior is as follows:
- The
timestamp
column on the session record in the database is refreshed when the site is used (provided that the user hits an uncached route from time to time and provided that this happens in a time window between$settings['session_write_interval']
andgc_maxlifetime
#17 - The expiry date on the cookie is never refreshed. #18 and #19.
If that is true, then I'd recommend to keep cookie_lifetime
values high and only reduce the parameters which govern the session garbage collection for sites which require expiration of idle sessions.
#37 links to the wrong CR. I think it should be that one → . And the CR links back to the wrong issue.
Also left a couple of comments in the MR.
As potential errors would occur when updating Drupal Core and not mailer we can't expect people to have read the documentation / release notes of the mailer module.
The contrib mailer module doesn't have a D11 compatible release. So sites using the contrib mailer wouldn't be able to update to Drupal 11 anyway yet. The experimental core mailer module will not be backported to any Drupal 10 release.
Thanks @stijn.blomme for your thoughts.
Do nothing
If the plan is to only keep the core module in experimental phase, and then move the code to the core framework, this could potentially never impact any website. If the users of the contrib mailer module keep the experimental core module disabled and do not use the new functionality before frame work inclusion.
This is more or less what I tested in #4. The important thing to realize here is that users of the contrib mailer
module do not have the choice to enable or disable the experimental core module. Modules need to have a unique name. And it looks like a module from contrib seems to override a module in core with the same name.
But honestly, as a site owner I wouldn't want to risk having two modules with the same name in my source tree. The findings in #4 should be treated as anecdotal. This certainly doesn't proof that two modules with the same name will coexist peacefully over the lifetime of a site.
Thanks to the recent discussions I think I'm starting to understand why @adamps and me are seemingly unable to agree on the broader architecture. I'll try to summarize what I believe are the two approaches, and I'll try to do that without any judgement. I believe the biggest difference lies in the scope of the call site.
Agent model
When sending a message, the call site gathers the minimum amount of information necessary to start the process. For a password reset, this would be the user entity exclusively. For a contact message, this would be the message entity, and only that. This information is passed on to an agent which knows how to build the body and subject, to whom to send it and in which language. The agent also decides who else is getting a copy and where replies are going to end up.
A message sending module provides a default agent for each type. The agent can be replaced with a custom one (satisfies [D2 CAN_REPLACE]
). Core ensures that it is easy to implement contrib and custom agents with consistent behavior. I.e., it should be straight forward to create a custom agent which invokes the same hooks / fires the same events as the default one. This could be implemented with an abstract base class (satisfies [D3 CAN_ALTER]
).
The agent model moves responsibilities from the call site into the agent implementation. E.g., all of the responsibilities of _user_mail_notify() will end up in some user mail agent. Also all of ContactMailHandler will end up in a contact mail agent. Same for the commerce OrderReceiptMail.
Controller/Template model
When sending an email, the call site gathers all the information necessary to build and send the message. E.g., to build a password reset, this includes the user entity, the fully qualified password reset url and the language. This information is passed on to a rendering component which invokes a controller and a template to generate the message subject and body. Finally, to send the password reset email, the call site gathers the account email address and sets it into the To
field and also populates the configurable repylTo
address.
An email sending module may provide any combination of default template and controller (also neither or both) for each type of message. An email sending module may also choose to reuse any combination of existing generic controller and existing generic template. The controller/template can be replaced by a custom one (satisfies [D2 CAN_REPLACE]
- on the presentation level which is less comprehensive than what the Agent model provides. In order to replace other parts of the message - e.g. addresses, a module would need to implement multiple alter hooks or transport events). The rendering component invokes the same hooks / fires the same events for every type of email (satisfies [D3 CAN_ALTER]
).
The controller/template model encourages separating the responsibilities between the call site (gathering the necessary data, populating addresses and other headers) and the controller/template (populating email subject and contents). It doesn't enforce it though.
Also the controller/template model operates on a lower abstraction level than the Agent model.
I tried what happens on a site where the contrib mailer
module is enabled during an upgrade. The contrib mailer module doesn't have a Drupal 11 release ready, so I had to patch it.
This is what I did:
Install Drupal 11 and drush:
composer create-project --no-interaction "drupal/recommended-project:^11" mailer-contrib-test
cd mailer-contrib-test
composer require -W drush/drush
Install the contrib mailer and patch for D11 compatibility:
composer require mglaman/composer-drupal-lenient
composer config --merge --json extra.drupal-lenient.allowed-list '["drupal/mailer"]'
composer require 'drupal/mailer:^2.0@beta'
sed -i 's/^core_version_requirement.*/core_version_requirement: ^10 | ^11/' web/modules/contrib/mailer/mailer.info.yml web/modules/contrib/mailer/modules/mailer_example/mailer_example.info.yml
Run a database and install the site:
podman run -d --rm --network=host --name=drupal_memory --tmpfs=/var/lib/pg/data -e PGDATA=/var/lib/pg/data -e POSTGRES_PASSWORD=postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=drupal docker.io/library/postgres:16
./vendor/bin/drush site-install --db-url=pgsql://postgres:postgres@localhost/drupal --no-interaction
Setup mailpit as sendmail binary:
chmod u=rwX,go=rX web/sites/default web/sites/default/*
echo "sendmail_path = ${HOME}/bin/mailpit sendmail" > web/sites/default/php.ini
export PHPRC="${PWD}/web/sites/default"
Enable the contrib mailer and the example module, generate login link, run the built-in webserver:
./vendor/bin/drush en mailer mailer_example
./vendor/bin/drush uli --no-browser --uri=http://localhost:8888
http://localhost:8888/user/reset/1/1755891039/btjHfkrOCd7x1nKu8KtRJ8DmHoNCpwxW548Ul0HLyJk/login
./vendor/bin/drush rs
Open the test page and mailpit in a second tab and confirm that mailer test mail gets delivered:
xdg-open 'http://localhost:8888/admin/config/system/mailer/example'
xdg-open 'http://localhost:8025/'
Upgrade the site to 11.x-dev:
composer require -W "drupal/core-recommended:^11.x-dev" "drupal/core-recipe-unpack:^11.x-dev" "drupal/core-project-message:^11.x-dev" "drupal/core-composer-scaffold:^11.x-dev" "drupal/core:11.x-dev"
./vendor/bin/drush updb
./vendor/bin/drush cr
Confirm that now there are two mailer modules in the filesystem:
find . -name mailer
Open the test form and do the test again.
Interestingly in my case after all these steps, things are still working as before. So, it looks like the contrib
mailer module wins the race for the \Drupal\mailer
namespace.
#16 touches on things which seem to be more the topic of #3534136-51: Requirements and strategy for Core Mailer → (answer is over there).
The comment over in #3539651-16: Introduce email yaml plugin and email controllers → raises interesting questions. Instead of answering directly, I'm going to rephrase this a little bit into a q/a format. Also I post it here, because it touches on strategy and architecture. I hope that this brings some clarity into the issue.
Q: Looking at the {module}.email.yml
file, it seems that the defaults
dictionary only contains a _controller
key most of the time. Why is it a dictionary in the first place? And is there a situation where more / other key pairs could be useful?
A: The defaults dictionary contains the default attributes for a certain email. The call site may override those using the $params
key when calling EmailRenderer:render
. So, basically the defaults
could specify additional defaults like, e.g., a view_mode
to pass to a generic controller which is capable of generating an email from any entity.
Q: Ok, so there could be a generic controller for entities. How would that look like in the {module}.email.yml
file?
A: Maybe something like this (not supported yet, but not difficult to implement):
my_module.my_email:
label: 'My custom email'
key: my_email
defaults:
_entity_view: 'entity_test.full'
Some hook_email_info_alter
implementation would, extract the entity type and view mode, add them as new defaults and also add a suitable _controller
.
Q: How would that work with attribute based plugins?
A: That could be implemented using traits or (abstract) base classes.
Q: What if the _controller
is missing and no hook_email_info_alter
cares to add one?
A: In that case, the email is rendered by just passing all the arguments to the template. For simple notification style emails its enough to specify an email in a custom module and add a template to the theme.
Q: Emails without "controllers", would that work with attribute based plugins as well?
A: Yes. Such plugin classes would probably end up being nearly empty.
Q: Does the controller in the controller design have the power to set a subject? Or a recipient? Or a custom header?
A: Yes, the controller has access to the Symfony email instance. It can set the subject (and if you look at the MR and the test classes, it does in most cases). And of course it runs in an isolated render context in the switched environment.
Q: In that case, the controller could control any aspect of the email?
A: Yes, it could. It has no opinion on where that stuff should happen and it doesn't force anything. As a consequence, contrib and custom developers can really do whatever they please.
Q: Some people seem to prefer attribute based plugins, other seem to prefer yaml based ones. How can we know which one is suited for this use case?
A: This is not easy to answer.
If we expect that in the future every email will require its very own implementation of that thing which has been hook_mail
for the last 13 years, then an attribute based plugin system is perfectly suitable.
If we expect that email in core takes a similar path as entities in core, and code sharing will get more and more common, then an attribute based plugin system is less useful. In the extreme case, we'd have to create empty plugin classes to put the attribute somewhere.
As pointed out by others (I guess it was @berdir), commerce already made a step in that direction. The commerce hook_mail
implementation is almost empty.
Oh, wow! Completely failed reviewing a one line patch, sorry for that. Looks right now.
Thanks.
I propose to add 📌 Resolve potential namespace conflict with contrib mailer module Active to the list of issues which need to be resolved before 11.3.
Tagging with 11.3 release priority. This needs to be resolved before release.
I managed to reduce the main branch in 📌 Introduce email yaml plugin and an email building pattern known from controllers Active to around 600 LOC (without tests). I'm going to update the issue summary over there to better reflect the current state.
I'm trying to reduce the scope of the Draft PR in order to make it smaller and easier to review. As a first step, I'll be extracting features which aren't absolutely necessary to land in the first iteration. I'm thinking of the following things:
- Support for controller closure (already removed)
- Customizable
Email
class (allows us to get rid of the custom plugin factory). - Move the builder code to another PR. While the builder simplifies the call-sites a lot, it doesn't need to be committed right away I think.
Anyone has additional suggestions?
I pushed a little refactoring of the renderer class. There is now an EmailControllerParams
object passed through the whole process. This gets rid of the repeated ad-hoc $context
definitions inside the render()
function.
It also separates the langcode
into an (alterable) environment parameters array. A hook_email_params
implementation now can actually modify the language of the mail. The language switching itself is not yet implemented though.
In the MR. There is a requirements check for unknown / misspelled password algorithms.
When an algorithm is configured which isn't available on the system, attempting to create new password hash will result in a fatal:
Fatal error: Uncaught ValueError: password_hash(): Argument #2 ($algo) must be a valid password hashing algorithm in Command line code on line 1
Verifying existing hashes still works (assuming the hashing algorithm is available on the system).
Re #8 / #9, the 3539651-core-emails
branch now contains a SystemEmailHooks
implementation (an example on how this could work):
https://git.drupalcode.org/issue/drupal-3539651/-/compare/11.x...3539651...
By the time this ships, every single site which upgrades will generate a requirements warning. The requirements warning points to a CR which contains example code for a service provider class. The example code for the service provider class is there because I think we shouldn't instruct people to manually add a container parameter for this. The reason I think so is that it is safer to use the \PASSWORD_*
constants instead of let people attempt to hard code the exact password algorithm into a yml file by hand.
Regarding the UI: This could be just a password_argon2
module with its own requirements check and a service provider which adds the password.algoritm: argon2id
container parameter.
Pushed the EmailBuilder
approach.
This code suggests that mail ID, parameters, and langcode will not be available at the point the email is being sent.
Good point. I think those should be added as tags / metadata to the Symfony Email object. This information is then also available on external mail service providers for filtering and routing.
Thanks for the review.
Since this has come up multiple times now, I guess I'm going to iterate on the call site now. There are a couple of well known design patterns which might help in this situation. In my eyes, the current MailManagerInterface
is a facade. It provides a convenience method (mail()
) which abstracts away the whole complexity of mail plugins, formatting and logging.
Looking through core, I can find another well known design pattern which simplify access to complex subsystems. E.g., EntityStorageInterface::getQuery()
returns an instance of a class implementing the builder pattern. The call site of entity queries resembles the example snippets by @zengenuity and @adamps. I think the builder pattern could be useful. E.g., a call site in the contact mail handler could look like this:
$key_prefix = $message->isPersonal() ? 'user' : 'page';
try {
$email = $this->emailManager->getEmailBuilder()
->id('contact.' . $key_prefix . '_mail')
->param('message', $message)
->langcode($recipientLangcode)
->build() # returns a Symfony Email
$this->mailer->send($email->to(...$recipients)->replyTo($message->getSenderMail()));
}
catch [...]
In cases where a message needs to be sent to multiple recipients with different languages, the builder can be reused (update module):
$builder = $this->emailManager->getEmailBuilder()
->id('update.status_notify')
->params($items);
foreach ($recipients as $recipient) {
[...]
try {
$email = $builder->langcode($langcode)->build();
$this->mailer->send($email->to($recipient));
$sent++;
}
catch [...]
In a second step, addressing, sending and exception logging could be moved into a convenience method. It doesn't need to cover all cases (build()
is still there) but it could simplify the most used ones:
Contact example:
$key_prefix = $message->isPersonal() ? 'user' : 'page';
$result = $this->emailManager->getEmailBuilder()
->id('contact.' . $key_prefix . '_mail')
->param('message', $message)
->langcode($recipientLangcode)
->send(to: $recipients, replyTo: $message->getSenderMail(), logger: $this->logger);
Update example:
$builder = $this->emailManager->getEmailBuilder()
->id('update.status_notify')
->params($items);
foreach ($recipients as $recipient) {
[...]
if ($builder->langcode($langcode)->send(to: $recipient)) {
$sent++;
}
Noted the other comments in #6. All of them are useful as well, thanks!
This is a very weird issue. These are my observations so far:
Before
📌
Avoid reading session from the database multiple times during a request
Needs work
(git switch --detach 4e26ae9cc3da8b8a90561716c4a53fdd6c07f8f6
):
- Session cookies are refreshed when big pipe module is enabled (e.g., standard install)
- Session cookies are not refreshed when big pipe module is not enabled (e.g., minimal install)
After
📌
Avoid reading session from the database multiple times during a request
Needs work
(git switch --detach 522404706e440106fe7e5d382ce018b0f89637d4
):
- Session cookies are not refreshed when big pipe module is enabled (e.g., standard install)
- Session cookies are not refreshed when big pipe module is not enabled (e.g., minimal install)
At least this is now consistently working as expected by @cilefen.
I realized, that the OP reported something different:
In 10.2.x, the session cookie is updated if necessary on every request (not yet sure whether this is done by PHP, Symfony or Drupal).
After
📌
Avoid reading session from the database multiple times during a request
Needs work
, the call to \Symfony\Component\HttpFoundation\Session\Session::save()
is too late to update the session cookie on big pipe responses. Response headers are already sent when big pipe performPostSendTasks()
is called. I can reproduce this behavior by adding a cookie_lifetime: 30
container parameter (and still using session_write_interval: 20
).
We could maybe fix that by adding more logic to \Drupal\Core\StackMiddleware\Session::handle()
. E.g., trigger a session save unconditionally when the session timestamp is older than session_write_interval
.
I tried to reproduce the issue with the following setup on 11.x
:
- Standard install profile.
- Create a new user without admin privileges.
- Login with the new user.
- Repeatedly reload a page which triggers big pipe (e.g.
/contact
).
In order to make the manual testing reproducible, I choose the following options for PHP session gc and Symfony session metadata_update_threshold (the latter is called session_write_interval
in Drupal and it is a setting):
In sites/default/services.yml
:
parameters:
session.storage.options:
gc_divisor: 1
gc_maxlifetime: 30
gc_probability: 1
In sites/default/settings.php
:
$settings['session_write_interval'] = 20;
The effect of the PHP gc_divisor: 1
and gc_probability: 1
is that gc is run on every request before any session data is read from the database. That means that any session record with a timestamp older than 30 seconds is removed at the start of ever request (except for requests answered by the page cache of course).
The effect of the metadata threshold is that a session write is enforced for sessions with a timestamp older than 20 seconds.
With this setup I can confirm that every request which hits the window between the metadata threshold (session_write_interval
and the gc max lifetime will bump the effective session lifetime by 30 seconds (gc_maxlifetime
). This is expected behavior.
I can confirm that a session is removed and a user is logged out when I leave a tab open for more than 30 seconds (gc_maxlifetime
) before I reload the browser (though, see the caveat at the end). That is expected behavior as well.
In my opinion this issue can be closed. I believe that the behavior observed by the OP is a result of a gc_maxlifetime
value lower/equal than session_write_interval
. If there is no window between the two values, the session timestamp is only updated when new session data is written. In cases where a very short session lifetime is required, it is probably best to set session_write_interval=0
. With that setting, sessions will be written on every request - no matter whether the session data has changed or not.
For the latter test, I discovered an interesting edge case. For the test to pass there needs to be other activity which triggers the gc in between the requests in the tab left open. If the session record is still in the database when the request starts, then the session record is removed from the database (which is expected), but it is written at the end again and as a result survives the timeout (which is unexpected).
I'm unsure whether or not this edge case needs a fix. With the probabilistic gc in use today, sites cannot rely on session records being deleted reliably after surpassing the gc max lifetime anyway.
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.
True. The call site needs more attention at some point. I'd like to keep the question open for a while to see what kind of abstraction best fits these cases. It very much depends also on whether or not we want to support passing a closure to $emailManager->getEmailDefinition()
.
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.
I tried to present many different approaches in 3539651-core-emails
. E.g., the confusingly named ActionMailBuilder
class is an action plugin and a mail builder at the same time (demoing the closure approach). The UpdateMailBuilder
shows that it is still possible to create text-only emails (question is whether we want to support that or not). UserNotificationHandler
shows the closure approach with first class callable syntax, ContactMailHandler
shows how it could look like if the "controller" is set via yml file.
If we want to continue with this approach for a bit longer, we should probably decide which pattern is to become the standard one.
The $langcode
is tricky and indeed merits an in-depth discussion. I think it is worthwhile to consider it thoroughly whether it should remain a required parameter (like it is today, and like it was on all my prototypes) or whether it should be dropped (as suggested by some comments). And if we decide to drop $langcode
from the call-site method signature, we should discuss that further with language subsystem maintainers I guess.
From the top of my head, I can think of at least four ways to determine the language for an outgoing mail:
- Use the site default language.
- Use the negotiated language (this only works if the mail sending is triggered in response to a browser request, it falls back to 1 on cron runs).
- Use the recipients preferred language (if the message has only one recipient and it is linked to a known user account).
- Use a language stored on a related entity (e.g., the language could be stored on a newsletter subscription).
It seems to me that the choice of language is context dependent. This could be an indicator that indeed the call-site is the most appropriate location to set the langcode. At the call-site, the negotiated language is still available (via ConfigurableLanguageManager::getCurrentLanguage()
). Depending on how the context switching is implemented, the negotiated language might return a different result inside the mail builder (i.e., inside the switched context). That in turn means, that mail builders have less options to choose from when attempting to determine the correct message language.
The call-site also might be aware of whether or not it is running in cron (update) or from within a form submission. Issues like 🐛 Password reset invalid mail notify language Needs work also indicate, that some users expectations are different than what others find intuitive.
Keeping the $langcode
on the call-site method signature forces developers to make a sensible choice. It is for sure debatable whether or not this is a good thing.
On the other hand, neither the old interface nor any of my prototypes has references to other i18n information. E.g., for certain messages it would be important to know the recipients timezone (e.g., calendar invites). So why do we have $langcode
as a required parameter on the call-site method signature but not $timezone
?
Added the requirement checks.
Should we add a config and a proper UI to make these requirement checks more actionable? Maybe an additional Password fieldset in admin/config/people/accounts
?
znerol → created an issue.
znerol → created an issue.
Any call to the old function continues to work as-is but doesn't go through the new API. It would mean they wouldn't use the new mail API once replaced, but that's perfectly fine IMHO, especially as long as the whole thing is still experimental.
Exactly. This is the responsibility of NotificationHandler::sendCompat()
. A replacement service implemented by the mailer
module either could override it (and throw an exception to make it explicit that _user_mail_notify() isn't supported by the experimental mailer). Or it could leave it alone and then a mail would be created and sent by the legacy mail system.
The individual functions are fine. They provide a good place for docs and we get rid of the $op
strings at the call site.
I updated the issue summary and the change record. This is ready from my side.
I went through a gitlab code search (
#3051266: Searching for code across all of Gitlab →
) in order find out how _user_mail_notify
is used in contrib. In order to exclude core clones and distros, I used the following query:
_user_mail_notify -path:core/modules/user -path:modules/contrib
That returns 228 results. I went through half of them and manually assessed how the function is being used. There are two bigger groups and a couple of interesting special cases.
Modules dealing with user registration
- https://www.drupal.org/project/better_register →
- https://www.drupal.org/project/bulk_user_registration →
- https://www.drupal.org/project/business_core →
- https://www.drupal.org/project/cas →
- https://www.drupal.org/project/civicrm_entity →
- https://www.drupal.org/project/cognito →
- https://www.drupal.org/project/commerce →
- https://www.drupal.org/project/commerce_guest_registration →
- https://www.drupal.org/project/created_account_register_message →
- https://www.drupal.org/project/crowd →
- https://www.drupal.org/project/entrasync →
- https://www.drupal.org/project/firebase_authentication →
- https://www.drupal.org/project/iyo →
- https://www.drupal.org/project/jsonapi_user_resources →
- https://www.drupal.org/project/minimal_register →
- https://www.drupal.org/project/nodehive_core →
- https://www.drupal.org/project/openlucius →
- https://www.drupal.org/project/opigno_learning_path →
- https://www.drupal.org/project/resend_register_mail →
- https://www.drupal.org/project/resend_user_login_link_user_operation →
- https://www.drupal.org/project/social_auth_hid →
- https://www.drupal.org/project/user_csv_import →
- https://www.drupal.org/project/webauthn →
- https://www.drupal.org/project/webform_user_registration →
Modules dealing with password resets
- https://www.drupal.org/project/admin_password_reset →
- https://www.drupal.org/project/diba_security_policy →
- https://www.drupal.org/project/eep →
- https://www.drupal.org/project/login_emailusername →
- https://www.drupal.org/project/login_flow →
- https://www.drupal.org/project/mass_password_change →
- https://www.drupal.org/project/mass_pwreset →
- https://www.drupal.org/project/openstory →
- https://www.drupal.org/project/pass_reset_checkbox →
- https://www.drupal.org/project/passwordless →
- https://www.drupal.org/project/patreon →
- https://www.drupal.org/project/rest_password →
- https://www.drupal.org/project/rest_register_verify_email →
- https://www.drupal.org/project/secure_password_reset →
- https://www.drupal.org/project/social →
General purpose / all ops
- https://www.drupal.org/project/if_then_else →
- https://www.drupal.org/project/mail_debugger →
- https://www.drupal.org/project/rules →
- https://www.drupal.org/project/user_account_emails →
- https://www.drupal.org/project/user_api →
Special cases
GSIS (GOV.GR) Login →
They are actually in the first group (Modules dealing with user registration) but they do just call $account->activate();
to trigger the registration email.
Own implementation
The following modules ship their own version of _user_pass_notify
.
- https://www.drupal.org/project/group_notifications →
- https://www.drupal.org/project/magic_code →
- https://www.drupal.org/project/user_registrationpassword →
Custom op
The Forgot username → module actually did implement a custom op (sight!).
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.
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".
Yes, of course.
% ./vendor/bin/drush updb
-------- ------------------ ------------- ------------------------------------
Module Update ID Type Description
-------- ------------------ ------------- ------------------------------------
system password_kernel_ post-update Rebuild the container after adding
parameters new kernel parameters.
-------- ------------------ ------------- ------------------------------------
┌ Do you wish to run the specified pending updates? ───────────┐
│ Yes │
└──────────────────────────────────────────────────────────────┘
> [notice] Update started: system_post_update_password_kernel_parameters
> [notice] Update completed: system_post_update_password_kernel_parameters
[success] Finished performing updates.
CR is ready. Also pushed an update to the StackedKernelPass
docs.
This is literally a beer idea, curtsy of @jurgenhaas → . I'm recording this here just for completeness:
If we think
📌
Introduce email yaml plugin and an email building pattern known from controllers
Active
one step further, email could just be one of the supported response formats. In order to render an email, create a Request
object with appropriate attributes, dispatch it through the http kernel as a subrequest, grab the email object from the response and send it via the transport framework to wherever its needed.
On the way through the request / response cycle, parameter upcasting, language negotiation, theme negotiation, render context isolation, render caching and maybe even cooperative multitasking via fibers is performed just like with every other request. Without experimentation its impossible to know what works and what breaks, though.
It would also be interesting to explore whether or not this code flow would simplify access checking on embedded attachments and images.
Considering email builders as routes (or microservices, if you prefer) also raises interesting questions. For example, some email builders likely are idempotent (simple notifications are always the same when generated with the same input) while others are not (the password reset link must be newly generated).
MaintenanceModeSubscriber
uses BareHtmlPageRenderer
nowadays.
I started working on a new approach for the core mail building infrastructure: 📌 Introduce email yaml plugin and an email building pattern known from controllers Active
I think this should feel quite familiar for people who are used to implement their own controllers.
In this iteration I also put more effort in the HTML/templating part. I took a look at the entity templates (node.html.twig, taxonomy-term.html.twig, media.html.twig, etc.). All of them use a contents
variable containing a list of content items. I think it would be good to use the same pattern in email.html.twig
.
I moved the language switching to the language
module. On monolingual sites this is now a no-op.
Also found a use case for this in core: HelpTopicSection::renderTopicForSearch().
Updated the deprecation message and the change record. _user_mail_notify()
is replaced by an internal service. It follows, that there is no replacement API, this must be reflected in the CR.
I restored the BC for _user_email_notify()
and also the return type behavior. Dispatching op to method call in _user_mail_notify()
is actually more work than keeping BC.
I also found a way to remove the interface. It is possible to mark methods for setter injection with the #[Required]
. That way, a subclass doesn't need receive and pass on a MailManager
via its constructor. It only needs to declare the services which it really needs.
I tried to update the issue summary in order to be more precise about the intention here. Regarding #20, this issue is about a different abstraction layer. It has no influence on the design of a future mail building layer.
Regarding the discussion whether this should be enums or separate methods:
🐛
Use email verification when changing user email addresses
Needs work
would definitely profit from separate methods. The new email address could be passed in via an additional argument. The current MR has to tunnel it via the param
array. This could be solved much better with a method with dedicated signature: https://git.drupalcode.org/project/drupal/-/merge_requests/5773/diffs#di...
Moving this under the umbrella of 🌱 [META] Adopt the symfony mailer component Active .
I removed the BC for custom $op
parameters. Instead _user_mail_notify
now maps the $op
to the correct method.
One thing to note is that this also changes the return value. Before, FALSE
was returned on error and NULL
when mail sending was suppressed (either by config or by a hook). The new methods return FALSE
in both cases.
Rebased for OOP hook conversion.
From the IS:
Don't re-use user_mail_notify or mutate the email on a cloned version of the user in case someone calls ::save() on it
I think this would be straight forward to implement after 📌 Extract _user_mail_notify() into a user MailHandler Active .
Issue 🐛 Use email verification when changing user email addresses Needs work seeks to add two new operations. It would be good to land this and the other issue in the same release.
Interesting observations in
🐛
_user_mail_notify return error when email sending is aborted
Active
regarding the _user_mail_notify()
return value.
📌
Improve documentation of _user_mail_notify
Postponed
regrettably ended up in a bike-shed discussion on whether it is necessary to update coding standards before committing the docs improvement. The MR in
✨
Use an enum for user email notification types
Needs work
tries to change every single occurrence of an $op
like string regardless of the context. By implementing the proposal from #2 (individual methods on a MailHandlerInterface
), it is possible to circumvent both of these pitfalls.
The MR here turns the $op
argument into an internal implementation detail. It isn't API anymore and therefore may remain a string.
#3463923-11: Improve documentation of _user_mail_notify → points out an interesting angle:
I must say I don't particularly like this, the enum would be much nicer. Regardless, I think it's possible (even if ill-advised) to alter or extend the available options, so this might be needlessly restrictive.
In Drupal 7 it was indeed very easy to extend the list of possible operations via a contrib or custom code. Simply by adding a pair of variable_set()
calls in an .install
file ensuring that user_mail_OP_subject
and user_mail_OP_body
contain some meaningful text. I haven't tried it, but it still might be possible to do something like this with a smart combination of form alters and config schema hacks.
This practice should therefore be deprecated properly.