Menu link edit & delete routes do not retain the current language

I have updated the IS to distill this down to a simple, reproducible issue.

Hello,

I'm providing a patch for this, but since this is a core issue, I'm not sure about the approach. I found two places where I can fix this, and chose the plugin because I thought it's more appropriate for reusability of the methods that I've updated.

The other place would have been in the \Drupal\menu_ui\MenuForm.php file

// Allow for a custom edit link per plugin.
$edit_route = $link->getEditRoute();
if ($edit_route) {
  // ----> Here I would have unset the language from the options array. <----
  $operations['edit']['url'] = $edit_route;
  // Bring the user back to the menu overview.
  $operations['edit']['query'] = $this->getDestinationArray();
}

But this second solution would apply only for the tree, so not everywhere, that's why I chose the first solution.

Your patch looks good to me @bbu23, although menu is not an area I know much about. This does need test coverage though, and I'm not sure if any test in the menu_link_content module is a good one to base the test on. There is also a test failure to fix.

Test cases for initial review.

Looks like unrelated failure.
Triggered the test again.

Uploading a test only patch to see if it fixes the issue.

As mentioned in #11 I have this test passed on my local.

I just tried to reproduce this on a minimal install of 9.1.x and failed. In step 6, the result I get is the expected result, the edit link points to
/it/admin/structure/menu/item/1/edit?destination=/it/admin/structure/menu. Since it was a minimal install there was no 'home' link to test as in step 5.

Ignore the above. I can reproduce it.

Having a go at this. I made a patch with the following changes.

1. +++ b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
   @@ -83,6 +84,18 @@ public function testTranslationLinkOnMenuEditForm() {
   +    $menu_link_content = MenuLinkContent::create([
   

   Don't think this is needed.

2. +++ b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
   @@ -83,6 +84,18 @@ public function testTranslationLinkOnMenuEditForm() {
   +      ['menu_link_content' => $menu_link_content_id], ['langcode' => 'fr'])->toString());
   ...
   +      ['menu_link_content' => $menu_link_content_id], ['langcode' => 'fr'])->toString());
   

   The docs for Url::fromRoute direct to \Drupal\Core\Url::fromUri() which doesn't have a 'langcode' property. It does have a language property which is an object.

The bug reported here is the "standard" practice throughout the whole entity system (e.g. you can observe this behavior for nodes too), so any change needs to be made on a more general level, not just for menu links.

@quietone
I've uploaded initial patch where we are modifying "getDefaultOperations()" method in EntityListBuilder class.

I was not able to use dependency injection because it will potentially affect all the classes that extends EntityListBuilder. It may affect many custom and contributed modules.
Triggering tests to evaluate all the failures and fix accordingly.

1. +++ b/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
   @@ -185,14 +185,16 @@ public function getDescription() {
   -    return $this->getEntity()->toUrl();
   +    $language = $this->languageManager->getCurrentLanguage();
   +    return $this->getEntity()->toUrl('edit-form', ['language' => $language]);
   

   Specifying the link type as "edit-form", when previously we used the default, seems like a significant change. Maybe I'm missing something.

2. We should add to the test coverage in EntityListBuilderTest.
3. IS needs updating for #18.

I set this to the wrong branch, moving back.

Hear an updated patch.

Fixing test case failures and adding entity creation for the menu link content entity.

Fixed issues in the patch.

I have tried to keep the changes restrained within scope of the issue, for this I have undo the changes at entity level files.

I disagree with the approach taken in #32, according to @amateescu in #18 we need to do this at the entity level instead of just for menu links.

So I think the next step here is to update the issue summary to make it more generalized and then we should try to continue on the patch in #30

#37 +1

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

have to agree with #37 so moving to NW for that work.