- Issue created by @benellefimostfa
- 🇹🇳Tunisia benellefimostfa
Attached a tested working patch, which removes the config_rewrite from the "Lupus Decoupled Menu" module:
- 🇹🇳Tunisia benellefimostfa
Removed config_rewrite dependency from .info as well.
- 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
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):
- It was added in 📌 Adopt config_rewrite for install time config Fixed == https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/43/d....
- The rest_menu_items.config.yml values are (and were) exactly the same as in the rest_menu_items module, so config-rewrite is not needed for that.
- The #3340799 MR also added a rest.resource.rest_menu_item.yml (but in config/install, not config/rewrite). I am guessing that the addition of config-rewrite might have something to do with that... but... that file is gone now. We don't need to support it anymore.
- Merge request !109Issue #3488399: Remove config Rewrite which is overriding Rest Menu Items allowed_menus config → (Open) created by benellefimostfa
- 🇸🇮Slovenia useernamee Ljubljana
@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 thelupus_decoupled_menu/config/rewrite
I'm not sure why it is even needed. All it does is that it removesadd_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.
- 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
@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.)