Allow hook_node_presave() to alter other node fields

This seems like a reasonable thing to do.

Can we get a test case in there that does exactly what you describe to verify that it's working as intended, so the next well-meaning person doesn't come along and move it back to the top again? :)

I feel that hook_node_presave() is superseded by hook_query_alter(). Why not removing it completely?

re #8: hook_node_presave() is used for 'nodeapi' steps that are relevant on both insert and update. I don't see how hook_query_later() deprecates it ?

Well, hook_query_alter() allows you to modify any query, insert or update, without having to have an explicit "presave" hook. I believe the former is broader then the later, and consequently deprecates it.

Well, in lots of cases, nodeapi modules do their stuff in their own tables, so what they might want to do in hook_node_presave() has nothing to do with the base {node} or {node_revision} queries ?

Well, in lots of cases, nodeapi modules do their stuff in their own tables, so what they might want to do in hook_node_presave() has nothing to do with the base {node} or {node_revision} queries ?

In that case, they do not need hook_node_presave(), but hook_node_update() and hook_node_insert()... The presave hook was designed for one thing only: to give modules the opportunity to alter the node object before part of it is saved to the node and node_revision tables. That looks a lot like hook_query_alter() to me.

re #14:
"give modules the opportunity to alter the node object before part of it is saved to the node and node_revision tables. That looks a lot like hook_query_alter() to me."
And depending on whether I want to modify a core element or a contrib addition, I need to find the right query and alter it ? Yuck.
Also: that works only if the node property is stored locally and saved by a db query.

I don't see why we'd want to prevent acting on the $node level while we can.

pre_save is not at all the same as query_alter. sometimes modules need to fiddle with node elements such as $node->og_groups in in helper modules. they might want to set the groups of a post according to own logic, and then all the saving happen as normal.

Sounds like #722688-21: Allow programmatic setting of node->changed's patch has some issues, there's talk of backing it out and resurrecting this issue.

Indeed, I have written a patch in that issue to back it out, and I am resurrecting this issue with a patch. Btw, I think this should be critical if the other is critical.

Forgot two new files. Tests will fail in the last patch. Re-roll.

+++ modules/node/node.module
@@ -994,6 +992,19 @@ function node_save($node) {
+    // The changed timestamp is always updated for bookkeeping purposes (revisions, searching, ...)
+    $node->changed = REQUEST_TIME;

Comment doesn't wrap at 80 characters, otherwise looks RTBC. Should we mark the other issue as duplicate of this?

114 critical left. Go review some!

I've marked #722688: Allow programmatic setting of node->changed duplicate. Here is a re-roll.

Since this is the correct fix for #722688: Allow programmatic setting of node->changed I'm closing that other issue and reclassifying this as a bug.

Whoopsie on the crosspost.

Let's move the test into node_test.module instead of its own module. We're already testing other hooks there such as view.

There we go.

Thanks! Committed to HEAD.