Support link options on menu item

https://www.drupal.org/project/structure_sync/issues/3073810#comment-13215771 works me using Drupal 10.1.3, structure_sync 2.0.5 and menu_link_attributes 1.3.0
Thanks

We need to find a way to support any field provided by any module that extend our menus/taxo/blocks

Ideas are welcome

Hi everyone,

Thank you very much for raising this issue and for providing a very helpful patch, it's greatly appreciated.

We came across the same requirement: Supporting import and export of the data added by the Menu Link Attributes → (same as #10) with the structure_sync module, but instead of going for this patch which solves the issue for this particular use case, we tried the more generic approach suggested in related issue #2980893-13: New fields added to the menu cannot be exported → with its patch and documented how the data added by menu_link_attributes could be supported.

Louis (@louis-cuny), when you have some time, we would greatly appreciate if you could please try taking a look at this other ticket ✨ New fields added to the menu cannot be exported Needs review , testing/reviewing the corresponding patch and more particularly give us your feedback on the approach suggested to potentially support any added configuration data to Menu Link content items.
 

Otherwise, concerning this particular ticket, from a personal perspective, I think since 'link__options' is actually a core database field (and 'options' a core Menu Link item property), it would certainly make sense to support the 'options' config key for Menu Link items by default/out-of-the-box in the structure_sync module. It is probably used by many popular contrib modules, such as Menu Link Attributes → (#10), Font Awesome Menu Icons → (issue summary) and I guess many more.

In other words, I would tend to agree this patch should be part of module's code base and therefore considering merging/accepting MR!16 (after testing/review, as usual).

I allowed myself to make a few additional changes to the MR at #12, in particular:
- Removed the need to serialize/unserialize menu link options: it feels much nicer to store data with the following format:

    options:
      attributes:
        class:
          - 'test-class1a test-class2a'
        target: _blank

As opposed to a serialized string:

    options: 'a:1:{s:10:"attributes";a:2:{s:5:"class";a:1:{i:0;s:25:"test-class1a test-class2a";}s:6:"target";s:6:"_blank";}}'

Which also prompts PHPCS coding standards error:
unserialize() is insecure unless allowed classes are limited. Use a safe format like JSON or use the allowed_classes option.phpcs
- Added basic schema definition for the 'options' menu link items config key, as a sequence with unknown keys and types (could be string, sequence, mapping, integer, uuid, etc...). I'm not sure exactly, but it probably seems to be better at this point to avoid restricting too much the data types that could be added to the options array, allowing any other modules, whether contrib or custom, to add any types of values.
 

Lastly, concerning #11:

We need to find a way to support any field provided by any module that extend our menus/taxo/blocks

The approach suggested in related ticket mentioned above ( #2980893 ✨ New fields added to the menu cannot be exported Needs review ) takes a step towards allowing any module to extend the Structure Sync module to allow exporting/importing to/from config any additional data to Menu Link content items.
The same concept could most likely be used for Block Content and Taxonomy Terms, but before going any further, it would be good to confirm whether the approach is moving the module in the right direction, starting with Menu Link items.
 

In any case, we would greatly appreciate if you could please try testing the patches (this one and the other), give us your reviews and opinions on the different approaches suggested in these tickets.

Feel free to let us know at any point if you have any questions or concerns on any aspects of this patch or the ticket in general, we would surely be very happy to help.
Thanks again very much to everyone for your code contributions, testing, reviews and feedback.

Thank you for your contribution

I agree with everything you have said
I would say it's missing testing empty and non-empty option field value

Hi Louis (@louis-cuny),

Thank you very much for your prompt and very positive reply to this issue, it's greatly appreciated.

Quick follow-up on your last comment, now that we have working tests on GitLab CI:

I would say it's missing testing empty and non-empty option field value

I went ahead above at #15 and added Tests to take into account the added 'options' key and cover cases for empty and non-empty option field values, see the corresponding changes in !MR16.

Quite a few assertions were added:

• 83, currently in the 2.x branch (https://git.drupalcode.org/project/structure_sync/-/jobs/630681).
• 112, with the patch and added cases for the menu links (https://git.drupalcode.org/issue/structure_sync-3073810/-/jobs/638576).

 
At this point all the tests seem to pass, which is probably a good sign ✅:
https://git.drupalcode.org/issue/structure_sync-3073810/-/pipelines/77784

Once again, we would greatly appreciate if you could please try testing this patch (!MR16), give us your reviews and opinions on the assertions and test cases added to cover the changes in this ticket.

Feel free to let us know at any point if you have any questions or concerns on any of the changes in this ticket or the added tests in general, we would certainly be glad to help.
Thanks in advance for your feedback and replies.

Thank you so much for your contributions.
You are bringing good vibes to this project !

I reviewed the MR and changes, tests are passing, I merge

Thanks a lot Louis (@louis-cuny) for your prompt, positive feedback and for merging in the changes, it's greatly appreciated.
We're certainly very happy to find an active maintainer for this project :)

Otherwise, I think at this point the next "big" one is ✨ Move import/export logic into plugin Active , for which we should probably try shaping out the bare minimum of the overall structure based on plugins....
I haven't really started looking into this, but I suppose there are good examples around, such as search_api, migrate, etc....
Where we could probably find an example of modular and extensible structure based on plugins.
But I definitely agree the logic should be broken down and taken out of the Controllers, wherever possible.

Also, I've seen your reply at #2980893-15: New fields added to the menu cannot be exported → concerning the idea of adding hooks to support fields added by other modules and think it all sounds great! But it does sound like a lot of work though....

So I'm thinking, since we've got this feature (menu links 'options' key) in the module already, it "should" solve this use case for quite a lot of modules and users ("out-of-the-box").
Therefore, we might park this other feature for the time being and see if we could perhaps focus our development efforts on a more global refactoring with the plugins approach.

I'll give it a bit of thoughts when I have some time and get back in the corresponding tickets.

In any case, feel free to let us know if you have any comments, suggestions or ideas on how to best make the module evolve and fix other tickets in the issue queue, we would surely be glad to help.
Thank you very much for your help, reviews, feedback once again, and great reactivity !

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