Allow programmatic setting of node->changed

I think I remember adding a patch to do this elsewhere, but I don't think I'd be able to find it. It's a tiny change, doesn't break any APIs, RTBC pending test bot.

This makes sense to me, and is consistent with us doing the same thing to $node->created this release. I've hit this before, too, I believe.

Committed to HEAD.

$node->changed was the goal of #322759: Allow hook_node_presave() to alter other node fields but maybe this route is better and that one can be set to duplicate (and it didn't need a test which stalled my patch ;)

BAD core maintainer. NO banana!

Actually, moshe, what do you think? Coltrane's patch does indeed seem to be a more holistic way to deal with this, rather than one-offing certain properties.

The original approach in this issue (plus the equivalent change to preserve $node->uid) seems preferable to me. It's simpler (at least in our use case) just to pass the desired changed/uid in the node object rather than implement a presave hook to stuff them in later. Also, what if a prepare hook is keying off these values? Well, I guess the answer there is to implement the prepare hook rather than the presave hook....

I think if anything we should eventually moved the REQUEST_TIME hunks out of node_save() - so you'd do something like node_add_defaults($node); node_save($node); if you want those values added.

While reviewing #761482: Incorrect sorting in admin/content/node, I noticed that this patch introduced a serious error. Consider these lines:

// The changed timestamp is always updated for bookkeeping purposes (revisions, searching, ...)
if (empty($node->changed)) {
  $node->changed = REQUEST_TIME;
}

If saving a new node, $node->changed will be set to REQUEST_TIME (assuming no contrib code is programatically setting it).

If saving an existing node, $node->changed will not be updated to REQUEST_TIME, because $node->changed is not empty (although it should be updated, as documented in the code comment!).

I wrote one failing test for all to witness.

This is why tests are good!

As for actually fixing this problem, it looks like we need a different way of programmatically setting $node->changed. Perhaps:

if (isset($node->changed_new)) {
  $node->changed = $node->changed_new;
}
else {
  $node->changed = REQUEST_TIME;
}

So, if you want to programmatically set $node->changed, you set $node->changed_new (or pick your own attribute name, I don't care what we call it).

subscribe

Marked as duplicate #761482: Incorrect sorting in admin/content/node

Pretty sure this is only needed on insert so why not check ->is_new there?

If the people behind the first patch just want to set changed on insert, then that would be an elegant solution.

@catch Take a look to #761482: Incorrect sorting in admin/content/node
Setting on insert make no sense because changed == created on insert and could be only used in contrib.
But trouble when user updates node - The only possible way to query "real change timestamp" is query {node_revision}.timestamp

So right now we have broken admin/content/node

I think catch meant that the ability to programmatically set $node->changed is only needed on insert. So, I can imagine the code working like this:

if (!$node->is_new || empty($node->changed)) {
  $node->changed = REQUEST_TIME;
}

The ability to explicitly set the changed value on update is useful when migrating nodes from external data.

When node is edited then "Authored on" always populated with value

EDIT: not sure about "Authored on" because it's name is "date" so "changed" should be empty... but it's not

In #322759: Allow hook_node_presave() to alter other node fields the implementation was just to move the invocation of the presave hook lower, so changed could be altered by modules implementing the presave hook. This would solve mikeryan's specific use case.

As per #20, I think it may be best to revert the patch in the OP and explore #322759: Allow hook_node_presave() to alter other node fields as a way to address people's migration issues. (We should also commit some of my tests in #8 :).

Alright, I'm backing the original patch out and providing the functionality requested in the OP in #322759: Allow hook_node_presave() to alter other node fields.

A problem in the tests is that I can't assert that the "changed" timestamp has changed, because REQUEST_TIME is constant between tests. But that is not a commit-blocking problem.

Marking duplicate of #322759: Allow hook_node_presave() to alter other node fields, where I back out the original patch in this issue, and provide the functionality requested in this issue's OP.

Since this conflicts with #322759: Allow hook_node_presave() to alter other node fields and that other issue is RTBC apart from a comment nitpick, I'm marking this as duplicate.

So I reopen #761482-5: Incorrect sorting in admin/content/node