Avoid extra DB query per menu item

The core issue ✨ Set MenuLinkContent getEntity to public visibility Fixed to expose \Drupal\menu_link_content\Plugin\Menu\MenuLinkContent::getEntity() looks like it will finally happen which makes Option 1 the best approach.

This is to update the patch to work with the latest 2.x branch code. It also adds a more specific check for MenuLinkContentInterface instead of just EntityInterface because we are looking for a menu link content entity.

After experimenting a bit, it looks like we can further simplify code with a public MenuLinkContent::getEntity(). The entity retrieved via the method is already the correct translation, so we can skip the entity.repository call.

Adding an updated patch with the simplified logic.

Thank you, that's looking really good! I'm a bit afraid of things breaking, so would be nice to have automated test coverage. Then we can RTBC and merge this with confidence!

And perhaps use a MR instead?

I've tested #4 https://www.drupal.org/files/issues/2023-07-06/translatable_menu_link_ur... →

and there is an issue. It's calling a protected method "getEntity()" that causes WSOD.

$url = $link->getEntity()->link_override->first();

Eduardo Morales Alberti → made their first commit to this issue’s fork.

I really like this change! The code looks much simpler and the fact that we reduced SQL queries number of site is very important.

I share concern of @Anybody, that we may fall into some of regressions and unit tests can help reduce that risk. But I'd create a initial unit (rather kernel) test as separate tasks. This module was designed as quick and simple workaround in D8 times and I wanted to keep it that way. As we have D11 and it's still helpful, it's good idea to improve its quality. But not now, I'll do that as separate issue.

I did some small refactor of code, as patch failed to apply and did manual testing. I was no able to "break" module after this modification.

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

This change broke compatibility with D9 as the method being used is still protected (and there aren't any plans on changing that in D9).

Might be worth flagging it as such so automated composer updates running on D9 sites don't update to 2.1.1, throwing fatal errors all around...

OH! Thanks for reporting. I'll deploy fix today later

I pushed fix for the issue on D9. I also added very basic (just loading of service) Kernel test, hopefully it will be start for test suite that will solve problems like this. It's late for me today, but tomorrow I'll try do some additional tests and I'll push the next release that works on both D9 and D10.