Export menu doesn't sort item according to their position in menu tree

Here is the patch to sort the menu link content.

It retrieves first the menu tree using

$tree = \Drupal::menuTree()->load($menu_name, new \Drupal\Core\Menu\MenuTreeParameters());

And then a recursive function to flatten it and finally sort the links using this flattened array.

There may be a better function, but this one is working fine for us.

Fixed patch

Thank you for your contribution !

Could you make some adjustments please :
- dependency injection instead of \Drupal::menuTree()
- camelCase for variables instead of snake_case ex: $flat_menu_tree
- use strict comparision for if ($menuLink->uuid() == $uuid) {

Hi, I will work on above points.

Updated version of #3 patch

Thanks for you help @keshav.k but you only changed the camel case.

Here is an updated patch to fix all 3 points mentioned by @louis-cuny in #4

Regarding the dependency injection, I'm not 100% sure if it's the correct approach as we had a statically instantiated class MenuLinksController() so I called it like this (I got this idea from Ajax Comments module and it's used in several parts in Core):

    $menuLinksController = MenuLinksController::create(\Drupal::getContainer());
    $menuLinksController->exportMenuLinks($form, $form_state);

Hi @tostini,

o I called it like this (I got this idea from Ajax Comments module and it's used in several parts in Core):

If you are referring to this file
Ajax comments controller then i think that this controller uses normal DI. Searching through core I found 2 or 3 examples of the syntax you use in your last patch. But all are inside test files. So i think they are a special case.

The class StructureSyncHelper.php should be defined as a service like FieldSettingHelper is.

Yes it should, but we don't want to rewrite to much stuff to avoid breaking other issues patches
So, if @tostinni's approach is working fine. I am okay with it.

We will be able to use correct service declaration in a v3 of this module

@louis-cuny I agree with your idea. DI is complex. Like in the services.yml example above where you are injecting Drupal services into your own custom service.. @tostinni's is using the same services syntax used in the rest of the file. So seems safe to assume that it is safe to use even if not the ideal one.

@andrew.farquharson yes this was the code I was refering to.
As said @louis-cuny I guess we'll keep this for the moment and see during the rewrite how to implement this as a service.

I did the same test as @andrew.farquharson without the patch and it did work fine, maybe the issue is more precise than that but anyway. I'm merging because it is not breaking anything

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