Problem/Motivation

menu_link_content_uninstall() tries to delete all content menu links when it is uninstalled.

We now have validation in place that will never allow you to get that far, you have to delete the content first. There's still a problem with deleting content there, but we need a solution for other entity types as well and it will need to scale from the start.

Proposed resolution

Remove the file.

Remaining tasks

User interface changes

API changes

Original report by @catch

Follow-up from #2301317: MenuLinkNG part4: Conversion. This won't scale once you get past a couple of thousand menu links.

CommentFileSizeAuthor
#6 drupal8-remove-menu_link_content_uninstall-2312389-6.patch1.07 KBer.pushpinderrana
Support from Tag1 fosters the development of DrupalTag1 logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Version: 8.x-dev » 8.0.x-dev
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Issue tags: +Novice

These are content entities. With the uninstall protection, is it even still possible for this to be doing anything?

Maybe we can just remove this function now? If so, then that's a trivial novice task.

I'll ask in #2278017: When a content entity type providing module is uninstalled, the entities are not fully deleted, leaving broken reference for someone to confirm this.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

yes, the uninstall validator will prevent this to run, so it can be removed. We'll need generic entity field purging (in a batch) if we want that to work again.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: menu_link_content_uninstall() loads all menu link entities » Remove menu_link_content_uninstall()
Issue summary: View changes

Great, updated title and issue summary.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Priority: Major » Normal

This also makes it a normal bug I think.

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Active » Needs review
FileSize
drupal8-remove-menu_link_content_uninstall-2312389-6.patch1.07 KB

Removed the file.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Just ensured and we indeed have no tests for that.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: drupal8-remove-menu_link_content_uninstall-2312389-6.patch, failed testing.

Status: Needs work » Needs review

tadityar queued 6: drupal8-remove-menu_link_content_uninstall-2312389-6.patch for re-testing.

Palashvijay4O’s picture

Palashvijay4O CreditAttribution: Palashvijay4O commented
Status: Needs review » Reviewed & tested by the community

The #6 patch applies cleanly and doesn't break anything . RTBC++ !! :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Great catch!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed b3e58a6 on 8.0.x 
    Issue #2312389 by er.pushpinderrana, Berdir, fago: Remove...

Status: Fixed » Closed (fixed)

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