Problem/Motivation

Module won't enable.

Steps to reproduce

When enabling the module, I at met with this error in console:

[02-Jun-2022 15:08:44 UTC] PHP Fatal error: Uncaught TypeError: Drupal\menu_position\Menu\MenuPositionActiveTrail::__construct(): Argument #5 ($entity_type_manager) must be of type Drupal\Core\Entity\EntityTypeManagerInterface, Drupal\context\ContextManager given, called in /var/www/html/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php on line 262 and defined in /var/www/html/docroot/modules/contrib/menu_position/src/Menu/MenuPositionActiveTrail.php:48

CommentFileSizeAuthor
#18 interdiff.txt980 bytesjoelpittet
#18 3283837-18-service-decorator.patch7.36 KBjoelpittet
#17 interdiff.txt6.06 KBjoelpittet
#17 3283837-17-service-decorator.patch7.2 KBjoelpittet
#16 3283837-11-service-decorator.patch5.72 KBjoelpittet
#12 3283837-10-service-decorator.patch4.77 KBjoelpittet

Issue fork menu_position-3283837

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kevinquillen created an issue. See original summary.

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno commented
Title: Cannot enable module » MenuPositionActiveTrail::__construct(): Argument #5 ($entity_type_manager) must be of type Drupal\Core\Entity\EntityTypeManagerInterface, Drupal\context\ContextManager given
Version: 8.x-1.0-beta2 » 8.x-1.x-dev
apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno commented

Basing on MenuPositionServiceProvider and MenuActiveTrail::__construct(), the fifth argument should be the entity_type.manager service.

kevinquillen’s picture

kevinquillen CreditAttribution: kevinquillen at Velir commented

Yes, I am not sure where or how this is being overridden. Forgot to mention I am using Drupal 9.3.14.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Active » Postponed (maintainer needs more info)

Sorry for the delay but sounds like the context module is passing things in that aren't the right thing.
https://www.drupal.org/project/context

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

kevinquillen’s picture

kevinquillen CreditAttribution: kevinquillen at Velir commented

Very possible, this issue sounds like it could be related. #3343912: Context module: "Menu Reactions" will not work because menu.active_trail has a different menu service provider.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented

@Ranjit1032002 I see you made a commit, but I don't see it, what is your plan?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Postponed (maintainer needs more info) » Active

Sounds like context module and menu_position are both trying to replace/extend the same service 'menu.active_trail' with different args.

@kevinquillen, you're totally right in #7.

Altering the service provider sounds like a bad idea all the way around, but seems to be what is recommended by core
\Drupal\Core\Menu\MenuActiveTrail::getActiveLink:117

    // Note: this is a very simple implementation. If you need more control
    // over the return value, such as matching a prioritized list of menu names,
    // you should substitute your own implementation for the 'menu.active_trail'
    // service in the container.
    // The menu links coming from the storage are already sorted by depth,
    // weight and ID.

We can hide the error and hobble menu_position like this:
https://git.drupalcode.org/project/context/-/commit/45261a4e454887db279f...

Or be more forceful about the priority somehow maybe...

Any suggestions?

joelpittet credited BradJones.

joelpittet credited cmlara.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Active » Needs review
FileSize
3283837-10-service-decorator.patch4.77 KB

How's this for a solution? Moved the class from altering the service to a service decorator. Help from @cmlara and @BradJones via slack.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Title: MenuPositionActiveTrail::__construct(): Argument #5 ($entity_type_manager) must be of type Drupal\Core\Entity\EntityTypeManagerInterface, Drupal\context\ContextManager given » MenuPositionActiveTrail conflicts with other modules altering the MenuActiveTrail service

Re-titling

joelpittet credited bradjones1.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
FileSize
3283837-11-service-decorator.patch5.72 KB

Whoops, forgot to include the deleted service provider.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
FileSize
3283837-17-service-decorator.patch7.2 KB
interdiff.txt6.06 KB

The CacheCollector the MenuActiveTrail extends from is pretty tightly coupled so I extended from that as well (saves the implementation of the public methods in the decorated class).

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
FileSize
3283837-18-service-decorator.patch7.36 KB
interdiff.txt980 bytes

Forgot need routeMatch service, this is now working locally from some testing.

  • joelpittet committed dd7c5e15 on 8.x-1.x 
    Issue #3283837 by joelpittet, kevinquillen, apaderno, bradjones1, cmlara...
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Needs review » Fixed

I've committed this to the dev branch for a new release soon.

Status: Fixed » Closed (fixed)

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