hook_node_insert() and other node hooks have imprecise/misleading descriptions

The current description says:

Take action when a new node of any type is being inserted in the database.

Looking at node_save(), which is what is invoking hook_node_insert....

It looks like the sequence is:
- Update the node table in the database
- Call the content-type-specific hook_insert() function, if it exists (via node_invoke() )
- Save the fields via field_attach_insert()
- Invoke all hook_node_insert() hooks, independent of content type

So Eikaa is correct: hook_node_insert() is definitely invoked after all database saving.

Also, if we are going to fix this one to be more accurate, we should take a look at the other hook descriptions from node.api.php. Such as hook_node_delete(), hook_node_update(), etc. Many of them suffer from the same problem.

Here's a rather large patch to fix this up, fix a few formatting errors, and make the node hook doc headers consistent with each other.

One note: I standardized on using the phrase "Act on" if the hook should be modifying the $node object, and "Respond to" if the hook wouldn't be modifying the $node object. E.g. the load-related hooks can add information to $node, and the update-related hooks (such as hook_node_insert mentioned in this original issue report) are only responding after the node has already been saved to the database.

Great clean-up! Committed to HEAD.

Parts of this probably apply to 6.x too, eh?

I'm setting this back to D7 -- looks like I missed cleaning up hook_load() and perhaps hook_node_load() somehow.

OK, here's a cleanup patch for hook_load() and hook_node_load() doc. I remember now that I skipped those in the first patch, because I had to figure out all the entity loading stuff. Spent the time now... :)

#594192: Do not release Drupal 7 until api.drupal.org supports classes really needs to be addressed... api.drupal.org is really not going to be useful in D7 without classes being shown, cross-referenced with regular functions, etc. Sigh.

Nice work, whoever updated the testing status line for patches, by the way. !! I like being able to see "Queued for testing", etc. Much clearer than the previous interface.

Just a note: This still needs a review, then a port to Drupal 6.

Committed to CVS HEAD. Thanks!

Needs port to Drupal 6.
Patches in #2 (main) and #7) cleanup. And of course the hook names are different in D6.

Glad to attempt, need access to developer/hooks :)

You can post a patch instead of comitting the changes (which is probably a good idea anyway).

http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...

Display a node.

This is a hook used by node modules. It allows a module to define a custom method of displaying its nodes, usually by displaying extra information particular to that node type.

Not clear enough, imo.
The important point is that this hook is only called for the node type declared by this module, in hook_node_info(). This needs to be highlighted, because it is not what you would expect from a normal Drupal hook.

Looks like we need to reopen this for D7, as that hook isn't updated sufficiently, like the other ones were. See
http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...
for an example of a hook with the right text near the top:
"This hook is invoked only on the module that defines the node's content type (use hook_node_insert() to act on all node insertions)."

Here's a patch. I also took out the reference to node_example, since it's not present in the other node hooks any more either.

If this is committed, please send back to D6 again, thanks!

d8 at this point...

#18: 629518-18.patch queued for re-testing.

#18: 629518-18.patch queued for re-testing.

Looks like this needs a reroll. Probably a good novice project?

Rerolled against current HEAD.

#25: nodehooks-doc-629518-25.patch queued for re-testing.

#25: nodehooks-doc-629518-25.patch queued for re-testing.

#25: nodehooks-doc-629518-25.patch queued for re-testing.

reglogge: why did the patch review keep failing?

Anyway, thanks for the re-roll. Someone now needs to review the text in the patch (I can't, as I wrote the original).

@Jhodgdon: the testbot had problems to drop the test database. Nothing to do with the patch.

OK, this patch just needs a review by someone other than me then. Thanks!
reglogge - do you want to review it? The idea is to verify that the patched doc block conforms to our standards:
http://drupal.org/node/1354
and that the text is good (reflects behavior of hook/function).

#25: nodehooks-doc-629518-25.patch queued for re-testing.

Looks like it needs a reroll, then a review by someone other than me.

Gave rerolling the patch a shot.

Just a thought,
maybe this kind of "hook" should rather be refered to just as a "callback" instead?

That is a separate issue donquixote... We have always referred to these as "hooks", and named them "hook_" in the API docs, although you are quite correct that they aren't hooks in the usual sense. If you want to start a renaming campaign for them and other not-really-hooks in drupal, I'd support it (on a different issue) -- but only if we did so for all the not-really-hooks.

Anyway, meanwhile this patch looks good for D8 / backport to D7. Thanks jvns.

Actually, this whole change (several patches above) needs to be backported to 6.x too eventually. The hook docs for 6.x are in the Documentation git repository by the way.

Committed/pushed to 8.x, moving back to 7.x.

D7 patch.

Thanks! I checked and this is the only remaining hook in D7 that doesn't have the better language in it.

After committing, we should move this to the Documentation project and fix up the Drupal 6 versions of these hooks in a similar way (in d6, the hook docs are in the Documentation project git repository).

Committed and pushed to 7.x. Thanks!