Problem

If a node type is deleted on uninstall with node_type_delete, it will still show up as an option in Add content until caches are cleared. If you click on the link after deleting, you get a bunch of errors.

I figured this out while testing my patch for #1002164: The Book module can be uninstalled with nodes with a book node type still existing.

CommentFileSizeAuthor
#5 deleted_node_types_in_menu_until_cache_clear-1238704-5.patch921 bytesruth_delattre
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webmestre’s picture

webmestre CreditAttribution: webmestre commented

Same pb in D7 :-(

bleen’s picture

bleen CreditAttribution: bleen commented

So there are a couple of ways to tackle this ... but I'm not sure which is "best" - I am pretty sure that OPTION C is worst though :)

Thoughts?

OPTION A: rebuild menus on modules_uninstalled

add this to menu.module

function menu_modules_uninstalled($modules){
  menu_rebuild();
}

OPTION B: move menu_rebuild() (and node_type_rebuild()) to node_type_delete

Currently this code is in node_type_delete_confirm_submit():

  node_types_rebuild();
  menu_rebuild();

If we moved it to node_type_delete() then this bug should be sorted

OPTION C: document that modules that delete a node_type should also rebuild modules

In theory, maybe this is simply the responsibility of the module ...

naxoc’s picture

naxoc CreditAttribution: naxoc commented

This seems to not be a problem anymore in D8. There is a patch in #1002164: The Book module can be uninstalled with nodes with a book node type still existing that makes the book content type go away from the Add Content page without having to rebuild the menu specifically (it probably happens somewhere else now).

naxoc’s picture

naxoc CreditAttribution: naxoc commented

Ah, the patch I talked about does not use node_type_delete().

I vote for option B then!

ruth_delattre’s picture

ruth_delattre CreditAttribution: ruth_delattre commented
Assigned: Unassigned » ruth_delattre
Status: Active » Needs review
FileSize
deleted_node_types_in_menu_until_cache_clear-1238704-5.patch921 bytes

Implemented Option-B

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Can we get some tests to show this is actually fixing the problem?

naxoc’s picture

naxoc CreditAttribution: naxoc commented
Issue summary: View changes

Patch does not apply anymore. I tried uninstalling the book module, and now the node type is never deleted no matter how many cache flushes are done.

ruth_delattre’s picture

ruth_delattre CreditAttribution: ruth_delattre commented
Assigned: ruth_delattre » Unassigned
iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu commented

I've tested this with standard installation profile (8.0.x-dev) and I was able to confirm that the links do not disappear:

  • Enable "Forum" module
  • Go to "/node/add" (you should see "Forum topic" content type)
  • Go to uninstall "Forum" module
  • Go to "/node/add" (you should not see "Forum topic, but it still is there)

Clearing caches does not make the forum content type disappear from "/node/add".

I did test this with user 1 and then I created also an test role and use with permissions "bypass node access", "administer content types" and "administer modules" and both has same result.

There is an test which should identify this and it's in \Drupal\node\Tests\NodeTypePersistenceTest which is weird.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Closed (cannot reproduce)

What you were testing is something else. It still showed that node type because it still existed. Uninstalling the moduel currently does not remove a node type.

Tested it using manually deleting the article node type. And it works as expected now, which makes sense because we never create per-node-type menu links anymore, the list on node/add is built dynamically.