Problem/Motivation

A relatively common use case is to alter and extend the menu form within the node form, but the hard part is altering the saved menu_link_content entity. It was possible with some tricks, but 11.5 made it significantly harder due to optimizations to not save the entity if no change is detected.

A typical example is the enabled flag ( #3075230: Provide menu link with disable option [Node Add Form]). That issue has been stuck for years and will likely be moved to menu_ui_extras. I'm proposing to add an alter hook to MenuUiUtility::menuUiNodeSave so other modules can alter the entity between the two complex phases of getting the entity in the correct translation/revision state and the checks to figure out if it should be saved or not.

The alternative is subclassing and duplicating that whole method, but that means maintaining 70 lines of complex code that will likely change again in the future. We also discussed refactoring the method into 2-3 separate protected methods, that would simplify subclassing but would still allow only one module to customize this. A hook seems like the more Drupal-like approach and in line with Drupal core strategy: Provide extension points instead of supporting all possible use cases.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3635291

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

berdir created an issue. See original summary.

berdir’s picture

Status: Active » Needs review

This is the simplification that this alter hook allows, and this approach was actually broken in 11.5:

   /**
-   * Implements hook_ENTITY_TYPE_presave().
+   * Implements hook_menu_ui_menu_link_content_build_alter().
    */
-  #[Hook('menu_link_content_presave')]
-  public function menuLinkContentPresave(EntityInterface $entity) {
-    // When saving a menu item, check if the node form defined an overridden
-    // status value.
-    // @see self::nodeFormSubmit()
-    $overridden_value = static::$menuLinkOverridenValue;
-    if ($overridden_value) {
-      /** @var \Drupal\menu_link_content\MenuLinkContentInterface $entity */
-      $entity->enabled->value = $overridden_value['enabled'];
-      static::$menuLinkOverridenValue = [];
-    }
+  #[Hook('menu_ui_menu_link_content_build_alter')]
+  public function menuLinkContentBuildAlter(MenuLinkContentInterface $menu_link_content, array $values, EntityInterface $parent) {
+    $menu_link_content->get('enabled')->value = $values['published'];
   }
 
   /**
@@ -102,21 +95,6 @@ public function formNodeFormAlter(&$form, FormStateInterface $form_state, $form_
         ],
       ];
     }
-    array_unshift($form['actions']['submit']['#submit'], [static::class, 'nodeFormSubmit']);
-  }
-
-  /**
-   * Custom form submission handler for the node form.
-   */
-  public static function nodeFormSubmit($form, FormStateInterface $form_state) {
-    $menu_values = $form_state->getValue('menu');
-    if (!empty($menu_values['enabled'])) {
-      // Override the published status of the menu link being saved/created.
-      // This saves in a static variable the current value of the checkbox we want
-      // to propagate into the actual `enabled` menu item field.
-      static::$menuLinkOverridenValue = ['enabled' => $menu_values['published']];
-    }
   }
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

We discussed this in slack some with me, you, and Darvanen and we agreed the hook was a good path. I made a very basic CR for it but no objections.

It's very basic that I thought maybe test would be overkill? But adding this seems to open the doors for a number of features potentially in core menu_ui, menu_ui_extras, and other contrib modules to more easily integrate.