Set MenuLinkContent getEntity to public visibility

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Question: Per #18 ✨ Set MenuLinkContent getEntity to public visibility Fixed

Also if the method is going to be public it should probably be added to MenuLinkContentInterface.

Is this accurate? We're looking at the menu link plugin class, which extends MenuLinkBase which implements MenuLinkInterface

Should it be on MenuLinkInterface or MenuLinkContentInterface?

I think the patch in #46 #47 #48 #49 #50 #51 seems incorrect re-rolls.
All are missing the new interface added in patch #40
Hiding all 5 patches.

- Adding a new test case to validate the getEntity method and validate the return value.
- CR added here https://www.drupal.org/node/3361300 →

Been 3 years without submaintainer review so moving to framework.

To get this in front of a committer for their thoughts.

+++ b/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php
@@ -9,11 +9,12 @@
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\menu_link_content\MenuLinkContentInterface as MenuLinkContentEntityInterface;
 
 /**
  * Provides the menu link plugin for content menu links.
  */
-class MenuLinkContent extends MenuLinkBase implements ContainerFactoryPluginInterface {
+class MenuLinkContent extends MenuLinkBase implements MenuLinkContentInterface, ContainerFactoryPluginInterface {
 

It's a bit confusing having two classes called MenuLinkContentEntityInterface. I don't think I realised this would happen when I asked for the method to be added to the interface.

Sorry to contradict myself from 2019, but I think we should not add this interface and just make the method on the plugin class public instead.

I checked for duplicate test coverage and couldn't find any, so that bit is fine.

Done, removed the additional interface and test is passing on local.

Oops, accidentally removed keyword.
Putting back.
Interdiff is against #54
Hiding ##5

OK this looks good.

It just being a public method on a plugin means there's no official bc guarantee, but in practice unless we do a major reworking of the menu links system again I can't see this changing dramatically, and for core I think it makes more sense than adding an interface for a plugin implementation.

Committed 7961aba and pushed to 11.x. Thanks!

Hi,

Thanks for the merge!

I think some info in the change record are wrong:
- Introduced in branch should be 11.x, not 10.2.x
- Introduced in version should be 11.0.0, not 10.2.0

Or am I confused with https://www.drupal.org/about/core/blog/new-drupal-core-branching-scheme-... → ?

10.2.x will be branched from 11.x later this year. I’m not sure if the introduced in branch should 11.x or 10.2.x but the version is correct.

Thanks for the quick reply!

So I was confused :)

I also searched for a 10.2.x branch in Gitlab in the meantime and did not find one, so this is the explanation.

As Drupal 10.1.0 was out, I thought a 10.2.x branch would already exist.

Sorry!

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