Automated Drupal 10 compatibility fixes

ModuleHandlerInterface::getImplementations() is deprecated and Drupal 10 provides no easy way to get a list of modules implementing a hook: https://www.drupal.org/node/3000490
This patch adds an hard-coded list for people that need to use this module on Drupal 10 and don't need other implementations, but we should find a new way to implement this.

I have reviewed and checked Patch #4 this works for drupal 9.5 and drupal 10.0. and 10.0.1
Screenshots of the compatibility on 9.5 and Config page on 10 have been attached for reference:

Drupal 10 is close so this is important.

But the patch introduces a new @todo and
core_version_requirement: ^8 || ^9 || ^10
is incorrect!

Must be
core_version_requirement: ^8.7.7 || ^9 || ^10
or
core_version_requirement: ^8.9 || ^9 || ^10

And please use merge-requests.

Thank you for maintaining the Generate Password module.
Hoping for a soft commit and a soft tag release. To ease testing with Drupal 10
Testing now with git clone git@git.drupal.org:project/genpass.git

Had Real physical testing round for Generate Password with Drupal ~10

composer create-project drupal/recommended-project:~10 /var/www/html/sandboxes/drupal10genpass__test
cd /var/www/html/sandboxes/drupal10genpass__test/
composer require drush/drush:~11.0;

cd /var/www/html/sandboxes/drupal10genpass__test/web/modules
git clone git@git.drupal.org:project/genpass.git
cd genpass/
git fetch origin
git checkout 8.x-1.x

Get the patch and apply it under Drupal 10 site. ( to be able to enable )

wget https://www.drupal.org/files/issues/2022-11-28/genpass-3287722-4.patch
git apply genpass-3287722-4.patch
rm genpass-3287722-4.patch

Change file/directory mod and ownership of files:

cd /var/www/html/sandboxes/drupal10genpass__test/
sudo chmod 775 -R .;sudo chown www-data:$USER -R .;

Install with Drush

./vendor/drush/drush/drush site:install standard --yes --site-name="Generate Password - Real physical testing with Drupal ~10" --account-name="webmaster" --account-pass="d" --account-mail="test@drupal.org" --db-url="mysql://root:123___@localhost/sandboxes_drupal10genpass__test" -vvv ;
./vendor/drush/drush/drush pm:enable genpass

[success] Successfully enabled: genpass

Rebuild the cache:

sudo chmod 775 -R .;sudo chown www-data:$USER -R .;
./vendor/drush/drush/drush cache:rebuild

Open a browser and change the address to:
http://localhost/sandboxes/drupal10genpass__test/web/

After a login with the webmaster test user
When creating a new testing user test.genpass for example
After saving the new user the following message will show up

Status message
Since you did not provide a password, it was generated automatically for this account.
Created a new user account for test.genpass. No email has been sent.

There's some unresolved feedback on the merge request from Anybody so moving this back to Needs Work.

The default works, but custom password implementations will not work as expected.

This is still "needs work."

Got that Greg, Thanks for having time to follow up on this issue.
Important notes from #12

Noticed Pierre's notes about

modules/contrib/genpass/genpass.module:
┌────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │                           MESSAGE                            │
├────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Fix    │ 323  │ Call to deprecated method getImplementations() of class      │
│ later  │      │ Drupal\Core\Extension\ModuleHandlerInterface. Deprecated in  │
│        │      │ drupal:9.4.0 and is removed from drupal:10.0.0. Instead you  │
│        │      │ should use ModuleHandlerInterface::invokeAllWith() for hook  │
│        │      │ invocations or you should use                                │
│        │      │ ModuleHandlerInterface::hasImplementations() to determine if │
│        │      │ hooks implementations exist.                                 │
│        │      │                                                              │
└────────┴──────┴──────────────────────────────────────────────────────────────┘

Oh the last code was a static return!

Researched a bit on what other modules are doing in this case
https://git.drupalcode.org/project/scheduler/-/commit/1bb684d#66fb3c85a3...

Got the point.
Testing custom module with a hook_password to be listed in the return of the genpass_algorithm_modules function.

/**
 * Return an array of all modules which implement hook_password.
 */
function genpass_algorithm_modules() {
  // Fetch a list of all modules which implement the password generation hook.
  // To be in this list, a module must implement a hook, and return random
  // passwords as strings.

  $return = [];
  \Drupal::moduleHandler()->invokeAllWith('password', function (callable $hook, string $module) use (&$return) {
    $return[$module] = $module;
  });
  return $return;
}

Not sure if I have a push access over the issue fork.
Hoping for Pierre to commit his suggested change.

It could be that invokeAllWith will invoke the functions.
To my understanding. In our case we only want to list not to invoke others.

All hook invocation delegated to Module Handler service
#2616814: Delegate all hook invocations to ModuleHandler

https://git.drupalcode.org/project/drupal/-/commit/8f1c7a8#9a1c5d1dd1901...

Use ModuleHandlerInterface::hasImplementations() to determine if hooks implementations exist.

Suggesting to change to use has Implementations:

/**
 * Return an array of all modules which implement hook_password.
 */
function genpass_algorithm_modules() {
  // Fetch a list of all modules which implement the password generation hook.
  // To be in this list, a module must implement a hook, and return random
  // passwords as strings.

  $all_modules = \Drupal::moduleHandler()->getModuleList();
  $return = [];
  foreach ($all_modules as $module_name => $module) {
    if (\Drupal::moduleHandler()->hasImplementations('password', $module_name)) {
      $return[$module_name] = $module_name;
    }
  }

  return $return;
}

If we are using moduleHandler()->hasImplementations, we can't keep the D8 compatibility, as this method was introduced in Drupal 9.4.

Furthermore, while running the tests locally, I got the following deprecation notice:

3x: user_password() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use \Drupal\Core\Password\PasswordGeneratorInterface::generate() instead. See https://www.drupal.org/node/3153113
3x in GenpassTest::testGenpassConfigsAndCreateUsersByAdmin from Drupal\Tests\genpass\FunctionalJavascript

But I can not seem to find any code using, user_password() and I have no idea what is causing this deprecation notice. Maybe one of the maintainers can rerun the tests using PHP >= 8.1, to see if that problem also appears on remote?

Nice, good point Joshua.
To me it feels a new 2.0.x branch would keep deprecated code and what new class/function/service to use. and support for PHP 8.1, PHP 8.2, later on.

• 2.0.x for ^9 || ^10
• 8.x-1.x for ^8 || ^9

• Ditching old ways is good
• Adopting new better ways is better

Managing between that is a tricky business
Let us see how the final changes will look like.

@Rajab Natshah, sounds good! :)

@greggles, could you perhaps create that 2.0.x as of #20 and #21? So we can proceed here :)

Hello @Anybody I have created 2.0.x branch in this issue fork. This is MR

f9be2491

What would the path to upgrade be for someone on Drupal 9 with this new 2.0.x that's proposed? Do they need to disable genpass as part of the core upgrade process?

Suggested to do this in two steps:

• 2.0.x for ^9 || ^10 ( only new code - ditching any ~8.0 or ~9.0 code using a logic with version compare !version_compare(\Drupal::VERSION, '10.0.0', 'lt')
• 8.x-1.x for ^8 || ^9 || ^10 ( with old code and new code - only needed for the upgraded process to 2.0.x )

• Drupal 8 and 9 sites could update to latest version on the genpass ~1.0
• then after the update to Drupal 10, only change in the composer from ~1.0 to ~2.0

• No disable for the module
• No drush updb on the switch from ~1.0 to ~2.0

New features could only be committed to 2.0.x
and freeze new features on the 8.x-1.x branch.

Hoping that this would be a good plan.
Greg, do you find this plan logical? for sure you know more needed cases to cover.

Thanks for laying that out, @Rajab Natshah. That mostly makes sense to me. 2.0.x being for 9 or 10 works for me, but I think you maintain a bit more compatibility than needed.

Drupal 8 and 9 sites could update to latest version on the genpass ~1.0
then after the update to Drupal 10, only change in the composer from ~1.0 to ~2.0

No disable for the module
No drush updb on the switch from ~1.0 to ~2.0

New features could only be committed to 2.0.x
and freeze new features on the 8.x-1.x branch.

That plan means that 2.0.x only needs to work with 10.x, right? That seems easier to do.

Another option:

Make 8.x-1.x compatible with both 8.x and 9.x (at this point I don't think we should invest much time in 8, honestly).
Make 2.0.x compatible with both 9.x and 10.x.
The upgrade flow is for 8.x-and 9.x to get on 9.x core with 8.x-1.x. I believe this is already possible and we can say that 8.x-1.x is no longer actively developed. Then once core is at 9.x people should upgrade to genpass 2.x and can upgrade core to 10.x.

Either of these 3 plans (yours, my first modification to yours to drop 10 support from 8.x-1.x branch, the 3rd one here to drop 10 support form 8.x-1.x and adjust the upgrade flow) works for me.

I'm also happy passing on maintainership if someone else wants to become maintainer - please file an issue with links to your activity in this module's queue showing creation of patches, reviews, etc. that demonstrate the work of a maintainer.

I am ok, with either keeping the 2.0x branch D9 and D10 compatible OR D10 only.

Although if we want to keep the D9 compatibility we have to define core_version_requirement: ^9.4 || ^10, as explained in #19:

If we are using moduleHandler()->hasImplementations, we can't keep the D8 compatibility, as this method was introduced in Drupal 9.4

Thanks, Greg, I agree that your modification plan is better.

Either of these 3 plans (yours, my first modification to yours to drop 10 support from 8.x-1.x branch, the 3rd one here to drop 10 support form 8.x-1.x and adjust the upgrade flow) works for me.

with Joshua's notes too.

2.0.x will have ^9.4 || ^10
8.x-1.x will have ^8 || ^9

Paving the way for a smooth upgrade workflow.

I'm also happy passing on maintainership if someone else wants to become maintainer - please file an issue with links to your activity in this module's queue showing creation of patches, reviews, etc. that demonstrate the work of a maintainer.

Love to help in maintaining the Generate Password module.
#3338506: Offering to Support and Maintain the Generate Password module

Greg, Thank you for time and consideration.

Starting with #3338638: Start a 2.0.x branch for the Generate Password module to support Drupal ^9.4 || ^10

As listed in #17

Having the following back to be committed to the 2.0.x branch.

Use ModuleHandlerInterface::hasImplementations() to determine if hooks implementations exist.

/**
 * Return an array of all modules which implement hook_password.
 */
function genpass_algorithm_modules() {
  // Fetch a list of all modules which implement the password generation hook.
  // To be in this list, a module must implement a hook, and return random
  // passwords as strings.

  $all_modules = \Drupal::moduleHandler()->getModuleList();
  $return = [];
  foreach ($all_modules as $module_name => $module) {
    if (\Drupal::moduleHandler()->hasImplementations('password', $module_name)) {
      $return[$module_name] = $module_name;
    }
  }

  return $return;
}

Thank you, Rajab for bringing this issue towards an end! :)

What are the remaining steps to get this committed to 2.0.x? As I am aware, we only need to test your changes (especially the "hasImplementations" part)?!

Or is there anything else to do, as you changed the status to "Needs work"?

Rebase failed: Rebase locally, resolve all conflicts, then push the branch. Try again.

#3338638: Start a 2.0.x branch for the Generate Password module to support Drupal ^9.4 || ^10

• ✅ Automated unit testing coverage
• ✅ Automated functional testing coverage

Development version: 2.0.x-dev updated 7 Feb 2023 at 09:35 UTC

• Testing result: PHP 8.1 & MySQL 8, D10.0.1 1 pass all results

Great work! Thanks for your time! :)

Thank you all, for the nice collaborative work on this issue for this important module.