Allow menu_link source plugin to filter menu links by menu name(s)

Can this be done already using Yaml? Eg, import only menu items in "navigation" or "user" menus:

  menu_name:
    source: menu_name
    plugin: skip_on_value
    not_equals: true
    method: row
    value:
      - navigation
      - user

Or look up the mapped value from the menu migration, and skip any menu items not allocated to a migrated menu:

  menu_name:
    -
      plugin: migration_lookup
      migration: my_menu
      source: menu_name
    -
      plugin: skip_on_empty
      method: row

I haven't tested these, and corrections are welcome, just checking if this plugin is necessary?

Alison your proposed PR above does have the advantage that it could exclude a large number of items for consideration by preventing them showing up in the source.

Agreed with @xurizaemon - this could be handled using the skip_on_value plugin.

I think this speaks to needing more HOWTO documentation on common scenarios to help people identify solutions that already exist.

I think this is a reasonable request and it can be a part of the default Drupal core plugin.
We already have similar conditions in Node and Term plugin, so why not follow the same pattern here?

skip_on_value is rather a workaround and not that useful for large datasets with many links.

I'm moving the issue under the Drupal core project and will open a merge request soon.

The first commit contains only test changes, so it failed as expected.
I also added a draft change record: https://www.drupal.org/node/3219548

This is ready for review 😊

Pushed one more commit with documentation improvements.

It might look a bit out of scope, but:
a) We need to edit the documentation of child classes to point that the parent class has some configuration keys;
b) We missed somehow adding documentation for menu link plugins in #3199741: Add documentation for remaining source plugins;

I reverted my previous changes to plugins documentation because it will be handled here: #3225227: Fix source plugin documentation.
Also, updated the IS a bit. Still needs review 👀

@Matroskeen, this looks really good. I noted in GitLab that the comment is repeated. Other than that it looks nice. The tests make sense. And the new code in the MenuLink source plugin follows the same pattern as in the Node source plugin, which is good. Things are consistent.

I tested the patch locally as follows. I applied the change to MenuLink and then ran a full migration from the drupal7 fixture. Everything was fine.

Then I updated the menu link migration to look like this:

source:
  plugin: menu_link
  menu_name: menu-test-menu
  constants:
    bundle: menu_link_content

I ran a fresh migration from the fixture and correctly saw that menu link content was migrated for the Test Menu but not for the Navigation menu.

Then I changed the menu_link migration again to read as follows:

source:
  plugin: menu_link
  menu_name: 
    - menu-test-menu
    - navigation
  constants:
    bundle: menu_link_content

This time the menu link content was correctly migrated for Test Menu and Navigation.

After the comment gets updated, this will be RTBC.

Whoah, no idea how that happened 😳
Thanks for the review! Hopefully, it's good to go now.

Ha! No worries. Looks good now.

Just noting this feature is not needed by core, which is the benchmark for adding features to migrate.

My two cents is that this case is fine, it is following existing patterns for adding a condition on a content source plugin. However, this does need tests, just some additional test cases to \Drupal\Tests\menu_link_content\Kernel\Plugin\migrate\source\MenuLinkTest should be fine. Setting to NW for that.

Also, I'll check with the other migrate maintainers.

@quietone, thanks for looking at this issue.

I agree that it's not needed by core, nor node_type property by Node source plugin, nor bundle by Term source plugin. However, it increases the DX and doesn't add too much complexity.

It would be partially covered when #3069776: SQL source plugins: allow defining conditions and join in migration yml lands, but we're not there yet. Besides, filters by bundle are essential and should be available in original source plugins in my opinion.

Additional tests cases are already available in \Drupal\Tests\menu_link_content\Kernel\Plugin\migrate\source\MenuLinkTest.
You can have a look: https://git.drupalcode.org/project/drupal/-/merge_requests/813/diffs#b12...

@Matroskeen, my apologies for missing that the test was there.

I reviewed the patch and the test both look fine. And nice to see an example included in the doc block. (That must be second nature now for you, Matroskeen).

There were no objections from any migrate maintainer for adding this to core. This is bringing menu in line with other source plugins that allow selection by bundle. As benjifisher pointed out it is 3 lines of 'dead simple' code with tests and documentation.

Therefore RTBC.

I think this is fine to make really simple - it does seem like a useful feature.

Committed 510b590 and pushed to 9.3.x. Thanks!