Problem/Motivation

When #86287: Password reset process ignores the user's language preference is committed there will be no places in core that provide a langcode argument to _user_mail_notify(). Therefore we should consider deprecating this argument.

Remaining tasks

User interface changes

None.

API changes

Deprecate langcode argument in _user_mail_notify().

Data model changes

None.

Release notes snippet

None.

CommentFileSizeAuthor
#7 3186752-7.patch3.19 KBlongwave
#3 3186752-3.patch1.36 KBravi.shankar

Comments

jonathanshaw created an issue. See original summary.

g-brodiei’s picture

g-brodiei
Primary language Chinese, Traditional
commented

Following for future deprecation work TODO.

ravi.shankar’s picture

ravi.shankar commented
Status: Active » Needs review
StatusFileSize
new 3186752-3.patch1.36 KB

Tried to address this issue, please review.

Status: Needs review » Needs work

The last submitted patch, 3: 3186752-3.patch, failed testing. View results

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

This is technically an internal API, but I suspect it should have a change record as it's ancient and useful and it's probably not uncommon for it to be called by custom code.

  1. +++ b/core/modules/user/user.module
@@ -1030,8 +1030,8 @@ function user_role_revoke_permissions($rid, array $permissions = []) {
+ *   language. The $langcode parameter is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0.

    See change record 

  2. +++ b/core/modules/user/user.module
@@ -1040,6 +1040,11 @@ function user_role_revoke_permissions($rid, array $permissions = []) {
+    @trigger_error('Language code to use for the notification, overriding account language is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0.', E_USER_DEPRECATED);

    I think this should be "Specifying the notification language using the $langcode parameter is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Omit the parameter. See https://..."

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

We have a test fail presumably due to this:

core/modules/user/src/Controller/UserAuthenticationController.php
262:      $mail = _user_mail_notify('password_reset', $account, $account->getPreferredLangcode());

So we still have one caller that sets $langcode - is this safe to simply remove?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3186752-7.patch3.19 KB

Added change record: https://www.drupal.org/node/3187082

Answering my own question in #6: yes, because the fallback is to the preferred langcode anyway.

Addressed #5 and #6 and added a deprecation test.

g-brodiei’s picture

g-brodiei
Primary language Chinese, Traditional
commented
Status: Needs review » Reviewed & tested by the community

1. Tried the test and it does push the deprecation message into the deprecation collections.
2. Test will fail when @trigger_error was commented out.
3. The message in deprecation annotation block and @trigger_error are both clear and easy to understand.
4. Confirms the removal of $langcode in _user_mail_notify in UserAuthenticationController.php does fall back to preferred Langcode by default.

Ref:
https://www.drupal.org/about/core/policies/core-change-policies/drupal-c...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed ab50d1b and pushed to 9.2.x. Thanks!

  • alexpott committed ab50d1b on 9.2.x 
    Issue #3186752 by longwave, ravi.shankar, jonathanshaw, g-brodiei:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.