I was wondering why my custom node type was always disabled after module installation. This is the reason:
The module uses _node_types_build() and NOT hook_node_info() to register the node type. And I wanted to change the base attribute to a module specific value other than the default 'node_content'. This combination causes the problem. Looking at the code of node.module : _node_types_build() line 712:

  // Check for node types from disabled modules and mark their types for removal.
    // Types defined by the node module in the database (rather than by a separate
    // module using hook_node_info) have a base value of 'node_content'. The isset()
    // check prevents errors on old (pre-Drupal 7) databases.
    if (isset($type_object->base) && $type_object->base != 'node_content' && empty($_node_types->types[$type_db])) {
      $type_object->disabled = TRUE;
    }

Types defined by the node module in the database (rather than by a separate module using hook_node_info) have a base value of 'node_content'
That is not true! node_type_save accepts values for base.

I don't exactly know what this 'feature' is for (some kind of garbage collection I guess), but I think this 'feature' should be remove because it is the modules' unstall rutines that should remove content types they created.

Comments

davidn’s picture

the module uses _node_types_build() and NOT hook_node_info()

was supposed to mean

the module node_type_save() and NOT hook_node_info()

sun’s picture

Title: modules' node types disable by _node_types_build() when using node_type_save & base != 'node_content' » Node types disabled by _node_types_build() when using node_type_save() with base != 'node_content'
Version: 7.2 » 8.x-dev
Issue tags: -disable, -node type, -base +Needs backport to D7
sun’s picture

Status: Needs work » Active
rbruhn’s picture

I found this after posting to #943588: node_type_delete(): hook_node_type_delete will fail for a disabled node_type. Trying to get property of non-object in hook_node_type_delete()

This is still presenting a problem. Though, in my circumstance the custom node type is enabled on install. No problems there.
The problem occurs when uninstalling. If the module is disabled, the node type is marked disabled in the node_type table. When _node_types_build() is called, it's not returned and causes:
"Notice: Trying to get property of non-object in comment_node_type_delete()"
The object $info in "module_invoke_all('node_type_delete', $info)" is empty.

Edit:
Appears to me the operations logic should be separated. Using _node_types_build() to rebuild active node types is fine and dandy, but during module uninstall there should be a flag, or different function, to collect disabled node types.

catch’s picture

tpopham’s picture

To the original poster, the node type is not removed as that comment in the code seems to imply, but is (intended) to set the node type's disabled column to 1 (see the node_type table). Having the type disabled subsequently removes it from a few UI's I think, probably explaining the coder's slip. Nothing to do with garbage collection.

I've been talking to myself over at #1268974: For disabled node types with 'base' != 'node_content', _node_types_build() does not return data about the type. about _node_types_build(). As I said over yonder, I think most devs have been treating `base==node_content' as a sentinel to make their node types work, with testing for other cases lacking. Ultimately, I'd like to see node_type_disable() and node_type_enable() functions defined, so that the hook_node_info() behavior that I expected (disabled module => disabled node type, with extant nodes left alone) could be implemented with the hook_module_enable() and hook_module_disable() hooks and hook_node_info() could go deprecated.

@rbruhn: I just posted a patch that I expect to break all kinds of stuff and perform poorly, but it's built more to demonstrate logic (I think that the _node_type_build() is used mostly from cache, though, judging by my drupal_set_message('peep')). The _node_type_build() function's return spec calls for both enabled and disabled node types. The patch remedied those "Notice: Trying to get property of non-object in comment_node_type_delete()" errors by returning the disabled node types also, but I expect a long list of failed tests (technically the function is private, but the public interface is ambiguous right now--I intend to fix that based on the results of testing). Currently in D7 you can disable Node Example and still create content of its type, and similar code is scattered all over the place, so I don't think many devs are obsessing over disable and uninstall cases (I can't imagine anyone could actually want that behavior, but changing it would break too much stuff at this point).

I used a drupal_load() and function_exists() to differentiate node types implemented by hook_node_info() from those implemented by node_type_save(), with the latter's disabled state never altered. Am I right, or should `base==node_content' be the only case that yields this behavior, and what about a hook_node_info() implementation with `base==node_content'? The tests are so thin right now, I think many cases could easily slip through.

The spec uses the term `active' often, but does it mean `there may exists nodes of this type floating around the database, but you might not be able to create any more until someone flips a switch' or does it mean `you can create nodes of this type'?

tpopham’s picture

Well I think I figured out the intended logic of _node_types_build() and have a patch awaiting review (and the automated tests were not thin at all...I think many of the tests exist for issues revolving around _node_types_build()). I don't have a Drupal 8 testing environment, so I'd love to get some feedback, and I think that patch may remedy this.
#1268974-7: For disabled node types with 'base' != 'node_content', _node_types_build() does not return data about the type.

catch’s picture

Status: Active » Closed (duplicate)

We don't need two issues for this, while this is the older one, #1268974: For disabled node types with 'base' != 'node_content', _node_types_build() does not return data about the type. has much more activity, marking this as duplicate.