Problem/Motivation

When installing "Lupus Decoupled Menu" module it triggers a config rewrite which replaces the "Rest Menu Items" configs.

The problem is that rest_menu_items 3.0.4 implemented a new config to only expose "allowed menus", which is overridden now by Lupus Decoupled Menu rewriting, therefore no menu is allowed to be exposed by default.

Steps to reproduce

- Update rest_menu_items to 3.0.4 then install "Lupus Decoupled Menu" (this only happens on first install).

Proposed resolution

- Either remove the config_rewrite "https://git.drupalcode.org/project/lupus_decoupled/-/blob/1.x/modules/lu..." if it's not needed as it was implemented to fix this issue https://www.drupal.org/project/lupus_decoupled/issues/3340799

- Or enable all the menus with code in lupus_decoupled_menu_install

I've tested removing the config/rewrite/rest_menu_items.config.yml and everything looked working fine, so I would suggest that solution if there are no objections from the main project maintainers.

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:

Comments

benellefimostfa created an issue. See original summary.

benellefimostfa’s picture

StatusFileSize
new620 bytes

Attached a tested working patch, which removes the config_rewrite from the "Lupus Decoupled Menu" module:

benellefimostfa’s picture

Status: Active » Needs review
benellefimostfa’s picture

StatusFileSize
new1.16 KB

Removed config_rewrite dependency from .info as well.

roderik’s picture

Status: Needs review » Needs work

Disregarding the fact that we need to have a MR instead of a patch, here:

We looked at this together and I agree that there seems to be no need for config_rewrite to be used (anymore) by this module, and it's just in the way (because it is rewriting new settings that the rest_menu_items has added, to NULL):

useernamee’s picture

@roderik here's my take on this:

I guess we could also slightly change the config_rewrite to replace only the output_values.

config_rewrite:
  replace: ["output_values"]

But since output_values are the same in the rest_menu_items/config/install and in the lupus_decoupled_menu/config/rewrite I'm not sure why it is even needed. All it does is that it removes

add_fragment: 1
base_url: ''
allowed_menus: {  }

from the rest_menu_items.config which doesn't look desirable.

That being said. If there's no need to change the values of above config options we don't even need the config_rewrite module in lupus_decoupled at all.

roderik’s picture

@useernamee we discussed this elsewhere but I can't find it. Summarizing just for reference:

  • true, I don't think we need the config_rewrite module at all.
  • Just realizing this now: we can and should remove config_rewrite from the composer.json because lupus_decoupled_menu is the only thing using it.
  • About getting rid of lupus_decoupled_menu completely: that may be well possible -- there are things in the .install file that seem like they are either unnecessary or can be moved into a recipe in the future -- but let's create a separate ticket for that if needed.

Current status of this ticket:

I think we all agree this is the way forward. However, our internal integration tests indicate that somehow,

  • the API response for menu items was being cached (2nd request is a cache hit)
  • with this change, it somehow isn't anymore.

It would be good to look into the reasons for that, so we're sure to know all implications. This ticket is blocked on that - which may take a little while.
(And also the MR comment.)

fago’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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