Problem/Motivation

I get a WSOD when trying to visit the settings page. The logged error is,

TypeError: Drupal\Core\Form\ConfigFormBase::__construct(): Argument #2 ($typedConfigManager) must be of type Drupal\Core\Config\TypedConfigManagerInterface, Drupal\Core\Extension\ModuleHandler given, called in /var/www/html/web/modules/contrib/ms_clarity/src/Form/MicrosoftClarityAdminSettingsForm.php on line 80 in Drupal\Core\Form\ConfigFormBase->__construct() (line 44 of /var/www/html/web/core/lib/Drupal/Core/Form/ConfigFormBase.php).

Steps to reproduce

Install module on a Drupal 11 site and try to visit /admin/config/services/microsoft_clarity.

Issue fork ms_clarity-3509189

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

keiserjb created an issue. See original summary.

keiserjb’s picture

keiserjb commented

The changes made to the admin form in the issue fork allow it to load successfully for me.

keiserjb opened merge request !3

kerasai’s picture

kerasai commented
Status: Active » Needs work

This patch does seem to fix the D11 compatability by addressing https://www.drupal.org/node/3404140 and https://www.drupal.org/node/3349759. The fixes look to be backwards compatible for D10 as well.

A few items possibly worth addressing:

These changes introduce some code standards/formatting violations.

1. Inline comments
2. Lost a docblock (::create method)
3. Whitespace added after a method’s closing bracket

Also, although calling Role::loadMultiple is noted in the change record where user_role_names() is deprecated, it’d be a more-appropriate solution to obtain the entity type manager service via dependency injection rather than calling the static method on the entity class.

One last thought, the constructor could be updated to use constructor property promotion, which is a cleaner solution versus the “normal” class properties and setter code in the constructor. We'll be seeing this more throughout core and contrib in the future.

kerasai’s picture

kerasai commented
Version: 2.0.1 » 2.x-dev
Status: Needs work » Needs review

Implemented changes as noted in #4 except the constructor property promotion, which I decided against as it does seem to be used elsewhere in the module.

etedal abu rajab made their first commit to this issue’s fork.

mkhamash’s picture

mkhamash commented
Status: Needs review » Reviewed & tested by the community

LGTM

ahmad-alyasaki’s picture

ahmad-alyasaki commented

Merged , Thanks everyone for your contributions!

ahmad-alyasaki’s picture

ahmad-alyasaki commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.