- ๐บ๐ธUnited States l-laziz
The feature/issue is still relevant for 4.x version and the patch #39 applies to 4.0.0 module version cleanly.
- ๐บ๐ธUnited States ShaunLaws
This patch does apply cleanly to 4.0.0, but caused a failure when I upgraded a site from D9 to D10.1.4. When running 'drush updb' it failed in password_policy_update_8305 at:
$config_path = drupal_get_path('module', 'password_policy') . '/config/install/password_policy.email.yml';
because drupal_get_path was removed from D10 - https://www.drupal.org/node/2940438 โ .
I suppose one upgrade path would be to apply the patch to the D9 site and run 'drush updb', but the path of least resistance for me was to re-roll the patch in #39 to change the failing statement to one which is compatible with both D9 and D10:
$config_path = \Drupal::service('extension.list.module')->getPath('password_policy') . '/config/install/password_policy.email.yml';
I have only tested the patch when running password_policy_update_8305 on a D10 site but it *should* work on D9 also.
- last update
about 1 year ago Composer require failure - last update
about 1 year ago 61 pass - ๐บ๐ธUnited States greggles Denver, Colorado, USA
This feature seems pretty unrelated to the purpose of the module.
- ๐จ๐ฆCanada Simon-P Vancouver
As the person who submitted issue https://www.drupal.org/project/password_policy/issues/3292927 โจ Customisation of email notification Active I have to say it it seems very related.
Here is an example of what our currently staff receive:
The text 'your password will expire soon' does not give any indication of what password needs changing, or even that it is a website password. If the person receiving it does not know which password needs changing they are unlikely to do it, which defeats the purpose of the module.
Given that password reset reminders are a commonly used in spam, people are going to ignore the email if there is no specific text that helps them identify that the email is genuine.
- Assigned to Kristen Pol
- ๐บ๐ธUnited States Kristen Pol Santa Cruz, CA, USA
Assigning to myself as I'm reviewing/merging ready RTBC fixes/updates over the next few days.
- ๐บ๐ธUnited States Kristen Pol Santa Cruz, CA, USA
Thanks everyone for working on this issue. I haven't read all the comments yet but regarding #44:
This feature seems pretty unrelated to the purpose of the module.
The email feature is already part of this module, so making the text configurable like other core user emails makes sense to me.
There are lot of patches and MRs updates so this is a bit hard to follow.
For example, it doesn't look like the latest patch included the fix noted in #26.
I'll look a bit more but this probably will need more work.
- Issue was unassigned.
- Status changed to Needs work
10 months ago 8:31pm 11 February 2024 - ๐บ๐ธUnited States Kristen Pol Santa Cruz, CA, USA
I've reviewed the code, but didn't test anything. Moving back to "needs work" based on the following:
Ignore my note above about #26. I thought the logic was reversed, so it looks okay.
One thing I noticed was that core uses
type: mail
in the schema, but that's not used in this code, e.g.user.mail: type: config_object label: 'Email settings' mapping: cancel_confirm: type: mail label: 'Account cancellation confirmation' password_reset: type: mail label: 'Password recovery' register_admin_created: type: mail label: 'Account created by administrator' register_no_approval_required: type: mail label: 'Registration confirmation (No approval required)' register_pending_approval: type: mail label: 'Registration confirmation (Pending approval)' register_pending_approval_admin: type: mail label: 'Admin (user awaiting approval)' status_activated: type: mail label: 'Account activation' status_blocked: type: mail label: 'Account blocked' status_canceled: type: mail label: 'Account cancelled'
-
+++ b/config/schema/password_policy.schema.yml @@ -57,3 +57,26 @@ password_policy.settings: +password_policy.email:
Nitpick: Inconsistent use of
email
vsmail
throughout code. Ideally, be consistent in usage throughout this code or be consistent with core naming if that is different. -
+++ b/config/schema/password_policy.schema.yml @@ -57,3 +57,26 @@ password_policy.settings: + label: 'Password to expire'
This label needs to change because this config object has both the "
expired
" and "to expire
" info. -
+++ b/password_policy.module @@ -13,6 +13,8 @@ use Drupal\user\UserInterface; use Drupal\datetime\Plugin\Field\FieldType\DateTimeItemInterface; use Drupal\Core\Url; +use Drupal\Component\Render\PlainTextOutput; +use Drupal\Core\Render\BubbleableMetadata;
Nitpick: Should be in alphabetical order.
-
+++ b/password_policy.module @@ -534,3 +543,97 @@ function password_policy_help($route_name, RouteMatchInterface $route_match) { + 'description' => t('Url of the site, no language prefix garanted'),
Wrong description.
-
+++ b/tests/src/Functional/PasswordExpiredEmailSendTest.php @@ -131,10 +131,10 @@ class PasswordExpiredEmailSendTest extends BrowserTestBase { + $this->assertMailString('body', "Your password will expire in less than $days_left days. Please visit the following link to reset your password: $link ", 1);
Empty space after
$link
. Is that expected? -
+++ b/tests/src/Functional/PasswordExpiredEmailSendTest.php @@ -147,10 +147,10 @@ class PasswordExpiredEmailSendTest extends BrowserTestBase { + $this->assertMailString('body', "Your password will expire in less than $days_left days. Please visit the following link to reset your password: $link ", 1);
Same.
Some other observations:
-
This patch failed to apply with version 4.0.2. So this is the reroll of the patch 2023_09_21_3240363-40.patch.
- ๐จ๐ฆCanada bdunphy
Patch in comment #50 works well so far on 4.0.3. Thank you to all who worked on this.