Import menu not available on MenuImportEvent

Hi @vasike,

Thanks for reporting this task.

Since we already have the target menu name available in the function where the MenuImportEvent is available, we can simply add it there to the event, and update the event class to expect this new argument.

While indeed the option to add it the item is also a possibility, I might prefer the object approach. As for the "source menu" we'll have to evaluate the options that we have or just rely on the exported information in the item. The menu_name value is supposed to be the exported menu (which is considered the source menu when imported) at that time. It's overridden a few lines later.

Though we should keep in mind that "source menu" is not a type of information that is mandatory and it's not something that applies globally to all plugins.

Sure, I understand that.

Yes, let's go for the Event, you can also add the source menu nullable if needed. And I think this should do it for this ticket*.

As for the rest of the discussion, I see the following:

Option to override the destination menu

At the moment, I don't see an immediate need for overriding the target menu. But I remain open to hear specific use cases that people might have in a dedicated ticket. Personally, I'd want to avoid it as much as possible, but who knows? As you said, some cases might arise.

Option to skip importing specific menu items

I do see scenarios where we'd want to skip some menu items, so I find this interesting. In fact, there were situations in the past where I would've liked to skip the deletion of specific items prior to import, but that would be a bit complicated to tackle in this ticket.

*The way I see it is: either we include this in the current ticket, and create a flag (using a method) in the Event to specify if the item should be skipped/process, and this would solve it for the import. Or if we want to develop something "bigger" then it could be done in a separate task.

Thanks @vasike, that looks correct though I don't know if you wanted to have also the source_menu or not. If you think the target menu is enough for now, that's good enough for me. But I do wonder if we should rename the property to targetMenu or something similar... 🤔

Regarding the pipeline extra failures, don't worry I'll take care of them.

Cool, sounds good.
I'll take it from here to solve the pipeline. Thank you!

Thank you for your contribution.

This will be included in version 4.1.0.

Pending change record & documentation updates.