node can be made a parent to itself

I noticed the same issue, but I found a different fix... maybe I'm missing something though, or we were working against different versions.

In the latest 7.x dev code:

function nodehierarchy_parent_selector_validate($element, &$form_state) {
  $selection = @$element['#items'][$element['#value']];
  if (is_array($selection) && !empty($selection['disabled'])) {
    form_error($element, t('You have selected a parent node which cannot be a parent of this node type.'));

    if (!empty($selection['nid']) && !empty($form_state['values']['nid']) && in_array($form_state['values']['nid'], nodehierarchy_get_node_ancestor_nids($selection['nid']))) {
      form_error($element, t('The parent of this node can not be itself or any if its decendants.'));
    }
  }
}

The if clause that checks ancestor nids only gets checked if the selected node is disabled (as a NH type).

I think it needs to be re-arranged like this:

function nodehierarchy_parent_selector_validate($element, &$form_state) {

  $selection = @$element['#items'][$element['#value']];
  if (is_array($selection)) {
    if (!empty($selection['disabled'])) {
      form_error($element, t('You have selected a parent node which cannot be a parent of this node type.'));
    }

    if (!empty($selection['nid']) && !empty($form_state['values']['nid']) && in_array($form_state['values']['nid'], nodehierarchy_get_node_ancestor_nids($selection['nid']))) {
      form_error($element, t('The parent of this node can not be itself or any if its decendants.'));
    }
  }
}

In my testing, this fixes the problem. Now, if you try to change a node's parent, and select the node or any of it's children, you get a clean error message.

I have attached a patch.

Seems to me like both #1 and #2 should be added to the module. The validation should definitely be fixed, but I don't see a reason for _nodehierarchy_tree_data() to return and display an 'un-choosable' node either.

Credit to solvita/garret.gengler; I just packed the two together (and got rid of the '@' in the validate function which I'm sure is against coding standards).

Wait, The patch in #1 needed some additions, IMHO:

• Comments. Because I started to doubt why nodehierarchy_get_node_ancestor_nids() is even called, if you are excluding the current node and its children from the tree anyway. (There is a reason, and it's commented now. Also, I split out the two 'if' conditions over two lines. That's partly just taste/readability.)
• Caching. Actually this whole module needs caching, but I couldn't stand nodehierarchy_get_node_ancestor_nids() being called in a loop like this.

This patch needs #2335219: Reduce database queries by caching data applied first. If you really don't want to do that, you can delete the 2nd hunk from the patch / the following two lines from the patched code:

@@ -1781,6 +1784,8 @@ function _nodehierarchy_parent_options($child_type, $exclude = NULL) {
  *   See menu_tree_page_data for a description of the data structure.
  */
 function nodehierarchy_tree_data($result = NULL, $exclude = NULL, $allowed_types, $depth = 1) {
+  // Prepopulate cache with one database query.
+  _nodehierarchy_get_menu_links();
   list(, $tree) = _nodehierarchy_tree_data($result, $exclude, $allowed_types, $depth);
   return $tree;
 }

I was still getting some invalid nodes in the list. See interdiff.
(This is not related to the original issue per se, but I just have too many needs-review patches to create a new issue for this...)