Deleted menus are not removed from content type config

Followed the steps and tried the same version (10.1.x-dev) to reproduce the issue, but it's working as expected.

Maybe here we can also tackle the empty third party setting

Clarified IS as it’s in the .yml config which I’m guessing was not clear. It is definitely happening.

Thanks, @pameeela, yes that point was not clear.

Didn't see a patch or open MR to review?

eashika → closed merge request !3478

eashika → reopened merge request !3478

Left a comment on the MR.

Also will require test coverage.

dcam → changed the visibility of the branch 11.x to hidden.

dcam → changed the visibility of the branch 10.1.x to hidden.

dcam → changed the visibility of the branch 3337882-deleted-menus-are to hidden.

The first MR took the wrong approach, which was removing the settings in the menu delete form submission step. But this would have left the settings in the event that a menu was deleted by some other means. That MR needed to be closed anyway since it was for a D10 branch. But the code for doing the setting removal was still fundamentally good even though it was put in the wrong place. So I copied it to a hook_ENTITY_TYPE_delete() implementation and used it as a starting point. Then I added a Kernel test.

I just realized that I messed up and removed the check for the parent setting being set to a specific link. I'll fix it later.

Hi, I've verified and tested MR! 11946 and applied the patch successfully on 11. x-dev Version: The changes are working as expected.

1. Create a New Menu Type: Go to /admin/structure/Menus/Add Menus, Give it a name (eg : Test Menu), Save.
2. Add the Menu to a Content Type: Go to Structure > Content types > Basic page > Manage fields>Edit >Menu settings, Under "Available menus", check your new Test Menu, Save
3. Delete the Menu: Go back to Structure > Menus, delete the Test Menu.
4. Check if Config is Still There: Go to Configuration > Development > Configuration Synchronization, Click Export, Click Single item, Choose Content type, Select Basic page, Click Export
5. Now open the exported YAML and check

Testing Result:
After you delete the menu, it will be removed from this YAML export automatically. Hence, the changes are working as expected.

Attaching the screenshot for reference.

Hence, moving to RTBC!
Thanks

Hi, I've verified and tested MR! 11946 on 11. 2-dev Drupal version.
Followed the same steps as #22, the changes are not coming as expected.

Testing Result:
After you delete the menu, it is still visible in this YAML export automatically.

Attaching the screenshot for reference.

Hi all, I've followed the steps to reproduce the issue and confirm that the issue persists.

1. Created a new Menu i.e., Explore Drupal.
2. Enabled the newly created menu to a content type under the Menu settings > Available menus
3. Exported the config using the drush cex command.
4. Now deleted the newly Menu.
5. Again, exported the config using the drush cex command.

Upon exporting the config for the first time, the new menu config has been exported and the content type config got modified with the changes in node.type.content_type_machine_name.yml file as follows under settings

third_party_settings:
  menu_ui:
    available_menus:
      - explore-drupal
      - main
      - tools
    parent: 'explore-drupal:'

and a new new config file "config/sync/system.menu.explore-drupal.yml" has been created for new menu.

After deleting the new menu, and exporting the configs with the same drush command did the following changes:

1. Deleted the config file "config/sync/system.menu.explore-drupal.yml"
2. However, the content type that were using the Menu, it's config file didn't get modify and the config remains the same.
   

   third_party_settings:
     menu_ui:
       available_menus:
         - explore-drupal
         - main
         - tools
       parent: 'explore-drupal:'

I applied the provided MR !11946 and it applied cleanly without any errors. I followed the same above steps and tried deleting the menu. However, on deleting the menu getting this errors:

The website encountered an unexpected error. Try again later.

Error: Call to a member function getStorage() on null in Drupal\menu_ui\Hook\MenuUiHooks->menuDelete() (line 262 of core/modules/menu_ui/src/Hook/MenuUiHooks.php).

Tested on Drupal 11.1.7

Since, the provided MR is not resolving the issue and some changes still required to resolve the issue hence, I'm moving it to Needs work. I'm attaching the screenshots of before and after fixes for reference. Additionally, I'm looking into the above encountered issue.

Thanks!

Hi, after debugging the error:
Warning: Undefined property: Drupal\menu_ui\Hook\MenuUiHooks::$entityTypeManager in Drupal\menu_ui\Hook\MenuUiHooks->menuDelete() (line 276 of core/modules/menu_ui/src/Hook/MenuUiHooks.php).
that I encountered while deleting the menu, I found that it was fixed in the issue Deleting a menu link from the node edit form does not apply hook_ENTITY_TYPE_access() 🐛 Deleting a menu link from the node edit form does not apply hook_ENTITY_TYPE_access() Active . The MR changes for the fixes were merged in the 11.x branch. Since I was testing on Drupal version 11.1.7, and its branch was 11.1.x, hence I was getting the error as the changes were not merged on the 11.1.x branch.

After applying both the MRs MR !11321 and MR !11946 as patch the encountered error is getting resolved. Now, on deleting menu, I'm not getting any error and also there's no error message appearing for the $entityTypeManager. After deleting the menu that has been enabled for a content type is also getting removed from the content type configuration upon exporting the configuration. I'm attaching the screenshots of after fixes for reference.

As the changes are working fine and MR is resolving the issue, and the pipeline was passing all the tests previously before pulling the latest changes from the source branch. Hence, I'm changing the issue status to RTBC.

Thanks!

Couple of questions on the MR.

I addressed the feedback with no edits. Restoring RTBC.

Thanks for the response - replied on the MR.

I addressed the feedback and made code changes.

Thanks the check looks right but we should inject ModuleHandler here now.

Thanks the check looks right but we should inject ModuleHandler here now.

I didn't do it initially because IIRC it's on the roadmap to split up these bulk Hook classes and wasn't sure if it mattered. Also, the procedural call is used throughout the other hooks. So I decided to err on the side of following the example of what was doing elsewhere in the class.

I took @berdir's suggestion of checking for the node_type definition so there's no new dependency.

Hi,

I have tested this ticket on the Drupal 11.x, and working as expected.

Steps followed:

1. Create a New Menu Type (/admin/structure/Menus/Add Menus), Save.
2. Add the Menu to a Content Type (Structure > Content types > Basic page > Manage fields>Edit >Menu settings)
3. Select the newly created menu from "Available menus", Save
4. Delete the newly created Menu (Structure > Menus)
5. Check if Config is Still There (Configuration > Development > Configuration Synchronization),
6. Go to Export > Single item, Choose Content type, Select Basic page, Click Export
7. Now open the exported YAML and check

After applying the patch successfully, able to see the menu is removed from config when it is deleted.

Hence moving this to RTBC+1

Attaching the screenshot for the reference.

::hasDefinition() is great, solves the same problem without the new dependency.

Note I'm not crediting various commenters here who manually tested the issue and posted screenshots - there is no visual bug to verify here, it's a config issue that already has automated test coverage and is easy to reproduce, so going through manual reproduction steps on top of that doesn't really help to fix the issue. Issues that need manual verification are usually CSS or markup issues where it's not easy to write automated test coverage and will usually have the 'needs screenshots' tag.

Committed/pushed to 11.x and cherry-picked to 11.2.x, thanks!

catch → closed merge request !11946