Problem/Motivation

The commit that fixed the issue in #3370924: \Drupal calls should be avoided in classes, use dependency injection instead seems to have caused an error. In that, cslevy comments:

Fatal error: Uncaught TypeError: Drupal\domain_language\DomainLanguageOverrider::__construct(): Argument #1 ($current_user) must be of type Drupal\Core\Session\AccountProxyInterface, Drupal\Core\Config\CachedStorage given, called in .../web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259 and defined in .../web/modules/contrib/domain_language/src/DomainLanguageOverrider.php:81

This class is defined in the services in the following way: 

  domain_language.overrider:
    class: Drupal\domain_language\DomainLanguageOverrider
    tags:
      - { name: config.factory.override, priority: -140 }
    arguments: ['@config.storage']
And also the create method was introduced. 
/**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    return new static(
      $container->get('current_user'),
      $container->get('domain.negotiator'),
      $container->get('config.factory'),
      $container->get('domain_config.overrider')
    );
  }
We don't need both. I think the dependencies should be defined in the services.yml, and this create method removed.
CommentFileSizeAuthor
#10 domain_language-3388388-10.patch3.63 KBmichael.acampora
#9 domain_language-3388388-9.patch3.2 KBmichael.acampora
#8 Screenshot 2024-01-19 at 5.21.41 PM.png17.78 KBbanoodle
#7 domain_language-3388388-7.patch3.71 KBbburg
#3 with patch.png444.38 KBkenyoowen
#3 applied patch.png66.1 KBkenyoowen
#3 without patch.png730.54 KBkenyoowen
#2 domain_language-3388388-2.patch1.89 KBbburg

Issue fork domain_language-3388388

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

bburg created an issue. See original summary.

bburg’s picture

bburg
he/him
Location Washington, DC
commented
Status: Active » Needs review
StatusFileSize
new domain_language-3388388-2.patch1.89 KB

Patch seems to prevent the errors. Not entirely sure if it fixes the original problem in the other issue.

kenyoowen’s picture

kenyoowen commented
Status: Needs review » Needs work
StatusFileSize
new without patch.png730.54 KB
new applied patch.png66.1 KB
new with patch.png444.38 KB

Hi bburg

I reproduced the error by enabling the module and applied your patch but still it has errors. Please see the screenshots attached.

Thank you.

bburg’s picture

bburg
he/him
Location Washington, DC
commented
Status: Needs work » Needs review

KenyoOwen, I don't believe the error in your screenshot can be attributed to this issue per se, it seems related to this one #3005048: Circular reference detected for service "router.route_provider". But feel free to correct me if you think I'm wrong.

bburg’s picture

bburg
he/him
Location Washington, DC
commented
Title: DomainLanguageOverrider::__construct(): Argument #1 ($current_user) must be of type AccountProxyInterface » DomainLanguageOverrider service arguments
bburg’s picture

bburg
he/him
Location Washington, DC
commented

Not sure if it's related to the patch, but upon coming back to this same project after a few days, I am getting this error:

Fatal error: Uncaught Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "entity_type.manager", path: "user_last_access_subscriber -> entity_type.manager -> string_translation -> string_translator.locale.lookup -> config.factory -> domain_language.overrider -> domain.negotiator". in /var/www/html/web/core/lib/Drupal/Component/DependencyInjection/Container.php:147

domain.negotiator was one of the arguments added to the overrider class in this issue.

bburg’s picture

bburg
he/him
Location Washington, DC
commented
StatusFileSize
new domain_language-3388388-7.patch3.71 KB

To address the errors in my last comment, I've moved accessing the domain.negotiatior to it's own method, that conditionally checks if it's already available. I was just following the example agentrickard uses in #3005048-16: Circular reference detected for service "router.route_provider". Which seems to have stopped the error for me, although I don't really understand why...

banoodle’s picture

banoodle commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screenshot 2024-01-19 at 5.21.41 PM.png17.78 KB

Patch #7 works beautifully for me - thank you!

screenshot of config export showing new config

michael.acampora’s picture

michael.acampora commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new domain_language-3388388-9.patch3.2 KB

Attached patch fixes a deprecation for php 8.2 that came in with the last patch, it's a small fix and thanks for the patch in #7 @bburg.

Deprecated function: Creation of dynamic property Drupal\domain_language\DomainLanguageOverrider::$currrentUser is deprecated in Drupal\domain_language\DomainLanguageOverrider->initiateContext()

michael.acampora’s picture

michael.acampora commented
StatusFileSize
new domain_language-3388388-10.patch3.63 KB

Forgot to add the services part in #9, re-added in this patch.

guillaumepacilly made their first commit to this issue’s fork.

guillaumepacilly opened merge request !15

guillaumepacilly’s picture

guillaumepacilly commented

Applied patch modifications into a MR and cleaned up references and annotations.

rajeshreeputra made their first commit to this issue’s fork.

dtfabio’s picture

dtfabio commented

Hi everyone,

MR 15 fixed the problem for me on:

Core version 10.3.13
PHP version 8.1.31
MySQL version 8.0.4

Greetings,

Fabio

guiu.rocafort.ferrer’s picture

guiu.rocafort.ferrer
Primary language Spanish
Location Barcelona
commented
Status: Needs review » Reviewed & tested by the community

Tested it and works well

dieterholvoet made their first commit to this issue’s fork.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Reviewed & tested by the community » Fixed

Merged to 2.0.x-dev. Thanks!

Status: Fixed » Closed (fixed)

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